-
Notifications
You must be signed in to change notification settings - Fork 458
feat: Add ability to create a change request ignoring conflicts #6236
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Docker builds report
|
| title='Ignore Conflicts' | ||
| component={ | ||
| <Checkbox | ||
| label='Create this change request even if there is an existing one for the same feature' |
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.
This isn't really related to being able to create the change request, but more about what happens when it goes live at the scheduled date.
Without this boolean enabled, a scheduled change will fail, and send a notification to the author, if another change has been made to the flag between creation of the change request, and the time it is due to go live. With this boolean enabled, it will ignore the conflict and update the flag with the information it had at the point of creation.
Based on this, we should (a) only show this if the change request is scheduled for the future, and (b) update the wording here. Perhaps something like:
"Ignore any conflicting changes when this change goes live. If disabled, and another change is made to this flag before the live date, this change will fail and a notification will be sent to the author."
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 could imagine a scenario where people line up these change requests without scheduling and the process is just manual - in a case where they don't know exactly when the time frame is they just know its coming.
For example this could be for maintenance but they do not know exactly when they want to turn the feature back on.
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.
Sorry, yeah, it looks like the logic also applies there, for CRs that stay open for a while, and other changes happen in between, this would be the case too. In that scenario though, we'd probably want to have this checkbox on the 'publish' action, rather than when you create the change request itself.
The FE technically could handle that itself by checking if there are conflicts on an open CR, showing the checkbox and then sending 2 requests when the user hits publish - one to manually update the CR to set "ignore_conflicts": true, and the second to actually publish the CR.
What we'd probably want to do though is to add this same ignore_conflicts attribute to the payload on the publish endpoint.
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.
Right, so this isn't needed really at all when creating change requests? One thing I noticed was we check for when the change request appears unchanged, it seems this has raised a usecase where submitting an unchanged change request is valid. I think I should just remove this check all-together and move the ignore_conflicts to the publish step?
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.
Not quite - there are 2 use cases:
- Publishing a change request
- Creating a scheduled change request
For the scheduled change, we do need this on creation of the change request.
Agreed that we should remove the hard fail on if there are no changes, but we should perhaps just show a warning message instead?
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature!Changes
Allows user to specify that they wish to ignore conflicts when creating a change request, this is useful when scheduling multiple updates to a single flag. Checking this option will also skip checks detecting that the feature state is unchanged.
How did you test this code?
Created multiple change requests for a flag