Add supp. billing notification to view licence #943
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Location of banner is assumed so will need to be reviewed by the reviewing developer. |
jonathangoulding
force-pushed
the
feature-license-page-billing-notification-banner
branch
3 times, most recently
from
b9f9b3b to
b02e83b
Compare
jonathangoulding
requested review from
Cruikshanks,
robertparkinson and
Demwunz
Cruikshanks
requested changes
Apr 25, 2024
There was a problem hiding this comment.
All of these requested changes are just a result of not knowing 'our ways' i.e. how @Cruikshanks likes things when he is paying attention 😬 . So, this is a ❤️ start and PR on day 2; 💪🏅!
One final thing to mention. There is no issue with using conventional commits when working on the branch.
But the first commit needs to follow our conventions and I encourage folks to use all 50 chars to make it as readable and meaningful as possible.
So, for this I'd be looking for something more like
Add supp. billing notification to view licence
https://eaflood.atlassian.net/browse/WATER-4323
The legacy view licence page we are replacing displays a notification banner if the licence is flagged for supplementary billing. It could be flagged for PRESROC, SROC or both so the message has to reflect each possible state.
So, this means we need to do the same thing in our version of the view licence page.If you follow our guide, GitHub does some cool stuff
- Your first
--allow-emptycommit will become your PR title and description - When it comes to the squash & merge, assuming you haven't strayed off-piste that initial commit message will be there to use again
Anyway, any questions or issues let me know.
https://eaflood.atlassian.net/browse/WATER-4323 The legacy view licence page we are replacing displays a notification banner if the licence is flagged for supplementary billing. It could be flagged for PRESROC, SROC or both so the message has to reflect each possible state. So, this means we need to do the same thing in our version of the view licence page.
jonathangoulding
force-pushed
the
feature-license-page-billing-notification-banner
branch
from
33e1916 to
c768a3d
Compare
jonathangoulding
deleted the
feature-license-page-billing-notification-banner
branch
Cruikshanks
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://eaflood.atlassian.net/browse/WATER-4323
The legacy view licence page we are replacing displays a notification banner if the licence is flagged for supplementary billing. It could be flagged for PRESROC, SROC or both so the message has to reflect each possible state.
So, this means we need to do the same thing in our version of the view licence page.
PRESROC notification
SROC notification
Both charge scheme notifications