-
Notifications
You must be signed in to change notification settings - Fork 237
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
Upgrade notify client to version 5.1.0 #925
Upgrade notify client to version 5.1.0 #925
Conversation
It looks like this is going to require input from a content designer/technical writer to help users understand what to do if they update a prototype using Notify. |
22d44cd
to
18d6aeb
Compare
Thanks for raising the PR @idavidmcdonald and letting us know about the breaking change; sorry it's taken us a while to look into it! I'm going to try and see what we can do about incorporating this change into the next major release of the prototype kit, which should be coming up in the next quarter. |
I've looked at the example code on the 'Using GOV.UK Notify to prototype emails and text messages' page and it looks like that doesn't need to be changed. |
Thanks! Let us know if you need any particular assistance please! |
Discussed with @joelanman , we think the best way to approach the release notes is to add a line saying that we have updated the Notify client, and if users experience problems they should contact us. We don't think many users will be using the return values from the API calls. We recommend that users check the release notes when upgrading an existing prototype, so that should also catch that situation. |
@joelanman I've added some draft content to the changelog, feel free to edit it or ask a content designer to take a look at it. As we don't want to break the repo until closer to release time, I will approve but not merge, and move this to the blocked column for the project. |
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.
Changes look good to me, this ready to merge when the prototype kit team is ready.
No changes needed to the prototype documentation but this will mean that when a user integrates their code with Notify and needs to look at the Notify documentation, that the version of the Notify client they have installed is the one used for the Notify documentation. We don't publish older versions of the docs on our main website, although it will still be available in git.
2d08f24
to
0fa324e
Compare
Created a new PR in order to rebase this change: #1134 |
Notify have just released version 5.0.0 of the notifications-node-client which is a breaking change (https://github.com/alphagov/notifications-node-client/blob/master/CHANGELOG.md).
The main documentation (https://docs.notifications.service.gov.uk/node.html#node-js-client-documentation) for how to use the node client is based on version 5.0.0 and could cause some confusion for someone using an older version of the node client as the documentation would not be accurate for someone using say version 4.7.3. We do not publish older versions of the client documentation on our main site (although it is available in git).
Therefore this commit bumps the prototype kit to use version 5.0.0 so that if a user of the prototype kit needs to look at the documentation for how to use the Notify client, it will match the version of the client they have installed.
The prototype kit at a glance appears to be one of our biggest users of the node client (in terms of repos with it installed, although I realise many of those will not actually use the Notify functionality) and some users may not be 'node experts' so hopefully removing any possible confusion here will be valuable.
Done when
Details of breaking change