Add more guidance about safety of Gitaly bumps
What does this MR do and why?
Helps authors and reviewers get Gitaly version bumps merged, faster and with less risk.
Original discussion
Updating the Gitaly Ruby Gem is safe as long as two things are given:
- No definitions that have been removed in the version bump are in use on the Rails side.
- No definitions that have been added in the version bump become used without a feature flag.
The first one should be catched via specs, and we don't add any new usage sites here. So yes, this bump looks good to me, thanks!
This domain expert comment is super helpful, and I think it removes the need for a back and forth before approval.
thought (non-blocking): Should we have some kind of guideline/danger section/automation to prompt this kind of statement of reasoning on all similar MRs, upfront?
Author Maintainer
Thanks @mkozono!
This merge request requires coordination with gitaly deployments. Before merging this merge request we should verify that gitaly running in production already implements the new gRPC interface included here.
Failing to do so will introduce a [non backward compatible change]? (https://docs.gitlab.com/ee/development/multi_version_compatibility.html) during canary depoyment that can cause an incident.
- Identify the gitaly MR introducing the new interface
- Verify that the environment widget contains a
gprd
deploymentI suppose maybe this comment from Dangerbot could be updated to include more detail. Any thoughts @pks-gitlab?
Developer
@mkozono @dskim_gitlab That would be a great addition indeed. I ain't really got much of a clue where these messages are maintained, but I'd be happy to review any MR that does address this.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.