-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove specialist topics from email-alert-service #732
Remove specialist topics from email-alert-service #732
Conversation
The |
We've got some questions related to this PR, so those need to be answered first before it's merged, so I've marked it as "do not merge" until those are answered. More info in the comments in the linked ticket. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made a couple of notes here. But it's the same comment as on the email-alert-api PR (so I'll spare you from having to read it again!) - ie let's use valid tag types, not legacy ones.
01fb2db
to
bd20c3a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks Mat
bd20c3a
to
181dc33
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What
Remove specialist topics from the supported attributes in email-alert-service.
Please investigate and make a note on the PR about what the `:topic` field is referring to here: https://github.com/alphagov/email-alert-service/blob/53ce12e4ea8f5643dbb3051c8ec5c9651afa8b6b/lib/tasks/message_queues.rake#L10C45-L10C51
Why
Retiring specialist topics.
Trello card