Skip to content
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

Rename ChangeNote to Notification #1628

Merged
merged 1 commit into from Oct 4, 2019

Conversation

@huwd
Copy link
Contributor

commented Oct 4, 2019

Background

Change notes have been used in the application as a log of changes we will notify subscribers about.

Each ones also has an optional 'note' that content can use to describe the nature of the change to subscribers.

This has generated historic confusion for developers and makes these concepts difficult for new starters to instantly grasp.

Pain point example

Pain point here arose in relation to this ticket, when writing up the documentation there were several points of confusion around the difference between a change_note and note describing a change on a change note

Change

As 'Change notes' are really a log of notified changes. This PR renames all references to 'change_note' to "notification'

@huwd huwd self-assigned this Oct 4, 2019
@huwd huwd requested review from benthorner and koetsier Oct 4, 2019
@bevanloon bevanloon temporarily deployed to finder-frontend-pr-1628 Oct 4, 2019 Inactive
Copy link
Collaborator

left a comment

Thanks @huwd for putting this up - it's going to make this so much easier to communicate! Just a couple of things to sort before we get this in:

  • Can we link to a Trello card in the description? A good card would be the one we discussed earlier, where this pain point came up (yet again).
  • Can we collapse the two commits we have into one? It would also be helpful to copy the PR description into the commit message, so the reasoning is part of the codebase.

In case it helps: in a single commit PR, the commit message automatically becomes the PR description, which saves having to type it twice, or copy/paste it!

@huwd huwd force-pushed the rename_change_note_to_notification branch from 0de5f8b to 6334f1d Oct 4, 2019
@bevanloon bevanloon temporarily deployed to finder-frontend-pr-1628 Oct 4, 2019 Inactive
@huwd

This comment has been minimized.

Copy link
Contributor Author

commented Oct 4, 2019

Done, done and done!

Change notes have been used in the application as a log of changes we will notify subscribers about.
Each ones also has an optional 'note' that content can use to describe the nature of the change to subscribers.
This has generated historic confusion for developers and makes these concepts difficult for new starters to instantly grasp.
Copy link
Collaborator

left a comment

👍

@huwd huwd changed the title Rename change note to notification Rename ChangeNote to Notification Oct 4, 2019
@huwd huwd merged commit 06949d5 into master Oct 4, 2019
3 checks passed
3 checks passed
continuous-integration/jenkins/branch This commit looks good
Details
continuous-integration/jenkins/publishing-e2e-tests Publishing end-to-end tests succeeded on Jenkins
Details
continuous-integration/jenkins/security No security issues found
Details
@huwd huwd deleted the rename_change_note_to_notification branch Oct 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.