Hello.
TLDR: In MW core, RELEASE-NOTES is a cause of the majority of merge conflicts.
I propose to restructure it in a way that could let us do less manual conflict resolutions in Gerrit.
The problem
When working on MediaWiki core, all of us often need to update RELEASE-NOTES-XX file
in case some deprecations or breaking changes were made. While the change is under review,
other changes touching RELEASE-NOTES-XX are made, causing merge conflicts that need
to be resolved manually. Non insignificant portion of manual conflict resolutions are needed
exclusively due to RELEASE-NOTES.
This is not a surprise due to the structure of the notes file. It has several sections, with every
section being a list of individual notes. Since most of us only append lines to the end of the list,
it creates a contention.
The proposal
I propose to reorganize RELEASE-NOTES. We keep the sections, but within each section
we start the note with the name of the class that’s been touched, if it makes sense.
For example, instead of writing
* In AbstractBlock, the getTargetAndType() and getTarget() methods are
hard deprecated. Instead use getTargetName() and getTargetUserIdentity()
together with getType().
We would write
* AbstractBlock: the following method were hard-deprecated:
- getTarget() - use getTargetName() or getTargetUserIdentity()
- getTargetAndType() - use getType() with getTargetUserIdentity() or getTargetName()
After switching to a more standardized format, we sort individual notes alphabetically by class
name. Method names within messages need to be sorted alphabetically as well. This will make
an insertion point of new release notes more random, hopefully reducing the contention and
thus reducing the probability of a merge conflict. Additionally, I personally think this will look nicer
and will be easier to read for extension developers.
Not all the release notes can follow this pattern, and it’s ok. The point is not to be perfect, the
point is to try to be better.
Alternative solutions
We could write a custom merge driver for RELEASE-NOTES which always puts ‘ours’ before ’theirs’,
but I’m not sure the result will justify the investment.
Feedback
Please tell me what you think about this.
- Do you think it is a real problem?
- Do you think simply restructuring the notes with help?
- Do you think writing a custom merge driver is a way to go?
- Will you remember to follow the new structure if we agree on it even though it’s not really possible
to enforce with linters since not all the notes can follow this structure?
- Extension developers - do you care? Do you think alphabetically sorted RELEASE-NOTES will be
any easier for you to read?
Thank you.
Petr Pchelko.
Staff Software Engineer.
WMF Platform Engineering Team.
TLDR: In MW core, RELEASE-NOTES is a cause of the majority of merge conflicts.
I propose to restructure it in a way that could let us do less manual conflict resolutions in Gerrit.
The problem
When working on MediaWiki core, all of us often need to update RELEASE-NOTES-XX file
in case some deprecations or breaking changes were made. While the change is under review,
other changes touching RELEASE-NOTES-XX are made, causing merge conflicts that need
to be resolved manually. Non insignificant portion of manual conflict resolutions are needed
exclusively due to RELEASE-NOTES.
This is not a surprise due to the structure of the notes file. It has several sections, with every
section being a list of individual notes. Since most of us only append lines to the end of the list,
it creates a contention.
The proposal
I propose to reorganize RELEASE-NOTES. We keep the sections, but within each section
we start the note with the name of the class that’s been touched, if it makes sense.
For example, instead of writing
* In AbstractBlock, the getTargetAndType() and getTarget() methods are
hard deprecated. Instead use getTargetName() and getTargetUserIdentity()
together with getType().
We would write
* AbstractBlock: the following method were hard-deprecated:
- getTarget() - use getTargetName() or getTargetUserIdentity()
- getTargetAndType() - use getType() with getTargetUserIdentity() or getTargetName()
After switching to a more standardized format, we sort individual notes alphabetically by class
name. Method names within messages need to be sorted alphabetically as well. This will make
an insertion point of new release notes more random, hopefully reducing the contention and
thus reducing the probability of a merge conflict. Additionally, I personally think this will look nicer
and will be easier to read for extension developers.
Not all the release notes can follow this pattern, and it’s ok. The point is not to be perfect, the
point is to try to be better.
Alternative solutions
We could write a custom merge driver for RELEASE-NOTES which always puts ‘ours’ before ’theirs’,
but I’m not sure the result will justify the investment.
Feedback
Please tell me what you think about this.
- Do you think it is a real problem?
- Do you think simply restructuring the notes with help?
- Do you think writing a custom merge driver is a way to go?
- Will you remember to follow the new structure if we agree on it even though it’s not really possible
to enforce with linters since not all the notes can follow this structure?
- Extension developers - do you care? Do you think alphabetically sorted RELEASE-NOTES will be
any easier for you to read?
Thank you.
Petr Pchelko.
Staff Software Engineer.
WMF Platform Engineering Team.