diff mbox series

[v2,5/5] SubmittingPatches: simplify guidance for choosing a starting point

Message ID 5ec91d02b7ae767cc9b2395e1c0fa10e1424c0c5.1689314493.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Commit 0a02ca2383455e40cfc0b1c0818479c2d36fe6cd
Headers show
Series SubmittingPatches: clarify which branch to use | expand

Commit Message

Linus Arver July 14, 2023, 6:01 a.m. UTC
From: Linus Arver <linusa@google.com>

Background: The guidance to "base your work on the oldest branch that
your change is relevant to" was added in d0c26f0f56 (SubmittingPatches:
Add new section about what to base work on, 2010-04-19). That commit
also added the bullet points which describe the scenarios where one
would use one of the following named branches: "maint", "master",
"next", and "seen" ("pu" in the original as that was the name of this
branch before it was renamed, per 828197de8f (docs: adjust for the
recent rename of `pu` to `seen`, 2020-06-25)). The guidance was probably
taken from existing similar language introduced in the "Merge upwards"
section of gitworkflows in f948dd8992 (Documentation: add manpage about
workflows, 2008-10-19).

Summary: This change simplifies the guidance by pointing users to just
"maint" or "master". But it also gives an explanation of why that is
preferred and what is meant by preferring "older" branches (which might
be confusing to some because "old" here is meant in relative terms
between these named branches, not in terms of the age of the branches
themselves). We also add an example to illustrate why it would be a bad
idea to use "next" as a starting point, which may not be so obvious to
new contributors.

Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Linus Arver <linusa@google.com>
---
 Documentation/SubmittingPatches | 96 +++++++++++++++++++++++----------
 1 file changed, 68 insertions(+), 28 deletions(-)

Comments

Junio C Hamano July 14, 2023, 5:31 p.m. UTC | #1
"Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +There is one guiding principle for choosing the right starting point: in
> +general, always base your work on the oldest integration branch that
> +your change is relevant to (see "Merge upwards" in
> +linkgit:gitworkflows[7]).  What this principle means is that for the
> +vast majority of cases, the starting point for new work should be the
> +latest HEAD commit of `maint` or `master` based on the following cases:
> +
> +* If you are fixing bugs in the released version, use `maint` as the
> +  starting point (which may mean you have to fix things without using
> +  new API features on the cutting edge that recently appeared in
> +  `master` but were not available in the released version).

I think this is technically not optimal, but is good enough for the
sake of simplicity and brevity[*].

> +* Otherwise (such as if you are adding new features) use `master`.
> +
> +This also means that `next` or `seen` are inappropriate starting points
> +for your work, if you want your work to have a realistic chance of
> +graduating to `master`.  They are simply not designed to provide a
> +stable base for new work, because they are (by design) frequently
> +re-integrated with incoming patches on the mailing list and force-pushed
> +to replace previous versions of these branches.

"unstable" is not the primary reason why you shouldn't build on
'next'.  It is "your work, if queued on 'next', cannot be merged to
'master' without pulling all the other undercooked stuff still in
'next'", as you describe in the next paragraph.  But that is
different from being unstable.  I'd suggest to use something like
this instead:

	... not designed to be used as a base for new work---they
	are there only to make sure that topics in flight work well
	together.  A topic that is literally built on top of 'next'
	cannot be merged to 'master' without dragging all the other
	topics in 'next', some of which may not be ready.  In
	addition, `seen` is frequently re-integrated with incoming
	patches ...

> +For example, if you are making tree-wide changes, while somebody else is
> +also making their own tree-wide changes, your work may have severe
> +overlap with the other person's work.  This situation may tempt you to
> +use `next` as your starting point (because it would have the other
> +person's work included in it), but doing so would mean you'll not only
> +depend on the other person's work, but all the other random things from
> +other contributors that are already integrated into `next`.  And as soon
> +as `next` is updated with a new version, all of your work will need to
> +be rebased anyway in order for them to be cleanly applied by the
> +maintainer.
> +
> +Under truly exceptional circumstances where you absolutely must depend
> +on a select few topic branches that are already in `next` but not in
> +`master`, you may want to create your own custom base-branch by forking
> +`master` and merging the required topic branches to it. You could then
> +work on top of this base-branch.  But keep in mind that this base-branch
> +would only be known privately to you.  So when you are ready to send
> +your patches to the list, be sure to communicate how you created it in
> +your cover letter.  This critical piece of information would allow
> +others to recreate your base-branch on their end in order for them to
> +try out your work.

Very well written.

Thanks, will queue.


[Footnote]

 * An very old but still severe bug in tagged versions would want to
   be fixed ideally not on top of 'maint' but on top of the latest
   tagged version in the same maintenance track.  E.g. if the commit
   X introduced the bug, you may ask "git describe --contains X" the
   oldest version the commit appears in, say "v2.30.0-rc2-gXXXXXX".
   Then you would run "git checkout -b fix v2.30.9" to start the
   branch to fix it.
Linus Arver July 26, 2023, 1:36 a.m. UTC | #2
Junio C Hamano <gitster@pobox.com> writes:

> "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> +There is one guiding principle for choosing the right starting point: in
>> +general, always base your work on the oldest integration branch that
>> +your change is relevant to (see "Merge upwards" in
>> +linkgit:gitworkflows[7]).  What this principle means is that for the
>> +vast majority of cases, the starting point for new work should be the
>> +latest HEAD commit of `maint` or `master` based on the following cases:
>> +
>> +* If you are fixing bugs in the released version, use `maint` as the
>> +  starting point (which may mean you have to fix things without using
>> +  new API features on the cutting edge that recently appeared in
>> +  `master` but were not available in the released version).
>
> I think this is technically not optimal, but is good enough for the
> sake of simplicity and brevity[*].
> 
> [Footnote]
>
>  * An very old but still severe bug in tagged versions would want to
>    be fixed ideally not on top of 'maint' but on top of the latest
>    tagged version in the same maintenance track.  E.g. if the commit
>    X introduced the bug, you may ask "git describe --contains X" the
>    oldest version the commit appears in, say "v2.30.0-rc2-gXXXXXX".
>    Then you would run "git checkout -b fix v2.30.9" to start the
>    branch to fix it.

In this example, are we using v2.30.9 as a starting point, not v2.30.0
because v2.30.9 is the latest tagged version that is in 'maint'? 

I think this nugget of knowledge should be included in a v3 of this
series. Will update.

>> +* Otherwise (such as if you are adding new features) use `master`.
>> +
>> +This also means that `next` or `seen` are inappropriate starting points
>> +for your work, if you want your work to have a realistic chance of
>> +graduating to `master`.  They are simply not designed to provide a
>> +stable base for new work, because they are (by design) frequently
>> +re-integrated with incoming patches on the mailing list and force-pushed
>> +to replace previous versions of these branches.
>
> "unstable" is not the primary reason why you shouldn't build on
> 'next'.  It is "your work, if queued on 'next', cannot be merged to
> 'master' without pulling all the other undercooked stuff still in
> 'next'", as you describe in the next paragraph.  But that is
> different from being unstable.  I'd suggest to use something like
> this instead:
>
> 	... not designed to be used as a base for new work---they
> 	are there only to make sure that topics in flight work well
> 	together.  A topic that is literally built on top of 'next'
> 	cannot be merged to 'master' without dragging all the other
> 	topics in 'next', some of which may not be ready.  In
> 	addition, `seen` is frequently re-integrated with incoming
> 	patches ...

Will update. Thanks.
Linus Arver July 26, 2023, 2:31 a.m. UTC | #3
Linus Arver <linusa@google.com> writes:

>>  * An very old but still severe bug in tagged versions would want to
>>    be fixed ideally not on top of 'maint' but on top of the latest
>>    tagged version in the same maintenance track.  E.g. if the commit
>>    X introduced the bug, you may ask "git describe --contains X" the
>>    oldest version the commit appears in, say "v2.30.0-rc2-gXXXXXX".
>>    Then you would run "git checkout -b fix v2.30.9" to start the
>>    branch to fix it.
>
> In this example, are we using v2.30.9 as a starting point, not v2.30.0
> because v2.30.9 is the latest tagged version that is in 'maint'?

Answering my own question, I now see that there are branches named
`maint-2.30`, `maint-2.31`, etc. And `2.30.9` is the latest version in
the `maint-2.30` branch.
Junio C Hamano July 26, 2023, 4:41 a.m. UTC | #4
Linus Arver <linusa@google.com> writes:

>>  * An very old but still severe bug in tagged versions would want to
>>    be fixed ideally not on top of 'maint' but on top of the latest
>>    tagged version in the same maintenance track.  E.g. if the commit
>>    X introduced the bug, you may ask "git describe --contains X" the
>>    oldest version the commit appears in, say "v2.30.0-rc2-gXXXXXX".
>>    Then you would run "git checkout -b fix v2.30.9" to start the
>>    branch to fix it.
>
> In this example, are we using v2.30.9 as a starting point, not v2.30.0
> because v2.30.9 is the latest tagged version that is in 'maint'? 

Yes, the example assumes that the last maintenance release for
v2.30.x series is v2.30.9.  But this kind of fix happening is
sufficiently rare and I do not think regular contributors should
have to worry too much about it.  If the affected area had tons of
changes between v2.30.9 and 'master', a fix on such an old base
would require a lot of work merging upwards, adjusting to newer
codebase, and it only makes sense to go that length for high value
fixes (aka "security patch").  The rules that apply for such a fix
would be vastly different (e.g. the review may be done behind closed
doors with small number of reviewers, not on the public list).

> I think this nugget of knowledge should be included in a v3 of this
> series. Will update.

So, while it may help improve understanding of the philosophy behind
the regular procedure to know, I am not sure it is worth spending a
lot of lines to describe it when we are giving a piece of advice for
general "bugfix and/or new development".  Some bugs are simply not
worth the trouble of going back for more than two maintenance
tracks to fix.

Thanks.
diff mbox series

Patch

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 40467372c0d..f1774c91e9c 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -10,34 +10,74 @@  available which covers many of these same guidelines.
 [[choose-starting-point]]
 === Choose a starting point.
 
-In general, always base your work on the oldest branch that your
-change is relevant to.
-
-* A bugfix should be based on `maint` in general. If the bug is not
-  present in `maint`, base it on `master`. For a bug that's not yet
-  in `master`, find the topic that introduces the regression, and
-  base your work on the tip of the topic.
-
-* A new feature should be based on `master` in general. If the new
-  feature depends on other topics that are in `next`, but not in
-  `master`, fork a branch from the tip of `master`, merge these topics
-  to the branch, and work on that branch.  You can remind yourself of
-  how you prepared the base with `git log --first-parent master..`.
-
-* Corrections and enhancements to a topic not yet in `master` should
-  be based on the tip of that topic. If the topic has not been merged
-  to `next`, it's alright to add a note to squash minor corrections
-  into the series.
-
-* In the exceptional case that a new feature depends on several topics
-  not in `master`, start working on `next` or `seen` privately and
-  send out patches only for discussion. Once your new feature starts
-  to stabilize, you would have to rebase it (see the "depends on other
-  topics" above).
-
-To find the tip of a topic branch, run `git log --first-parent
-master..seen` and look for the merge commit. The second parent of this
-commit is the tip of the topic branch.
+As a preliminary step, you must first choose a starting point for your
+work. Typically this means choosing a branch, although technically
+speaking it is actually a particular commit (typically the HEAD, or tip,
+of the branch).
+
+There are several important branches to be aware of. Namely, there are
+four integration branches as discussed in linkgit:gitworkflows[7]:
+
+* maint
+* master
+* next
+* seen
+
+The branches lower on the list are typically descendants of the ones
+that come before it. For example, `maint` is an "older" branch than
+`master` because `master` usually has patches (commits) on top of
+`maint`.
+
+There are also "topic" branches, which contain work from other
+contributors.  Topic branches are created by the Git maintainer (in
+their fork) to organize the current set of incoming contributions on
+the mailing list, and are itemized in the regular "What's cooking in
+git.git" announcements.  To find the tip of a topic branch, run `git log
+--first-parent master..seen` and look for the merge commit. The second
+parent of this commit is the tip of the topic branch.
+
+There is one guiding principle for choosing the right starting point: in
+general, always base your work on the oldest integration branch that
+your change is relevant to (see "Merge upwards" in
+linkgit:gitworkflows[7]).  What this principle means is that for the
+vast majority of cases, the starting point for new work should be the
+latest HEAD commit of `maint` or `master` based on the following cases:
+
+* If you are fixing bugs in the released version, use `maint` as the
+  starting point (which may mean you have to fix things without using
+  new API features on the cutting edge that recently appeared in
+  `master` but were not available in the released version).
+
+* Otherwise (such as if you are adding new features) use `master`.
+
+This also means that `next` or `seen` are inappropriate starting points
+for your work, if you want your work to have a realistic chance of
+graduating to `master`.  They are simply not designed to provide a
+stable base for new work, because they are (by design) frequently
+re-integrated with incoming patches on the mailing list and force-pushed
+to replace previous versions of these branches.
+
+For example, if you are making tree-wide changes, while somebody else is
+also making their own tree-wide changes, your work may have severe
+overlap with the other person's work.  This situation may tempt you to
+use `next` as your starting point (because it would have the other
+person's work included in it), but doing so would mean you'll not only
+depend on the other person's work, but all the other random things from
+other contributors that are already integrated into `next`.  And as soon
+as `next` is updated with a new version, all of your work will need to
+be rebased anyway in order for them to be cleanly applied by the
+maintainer.
+
+Under truly exceptional circumstances where you absolutely must depend
+on a select few topic branches that are already in `next` but not in
+`master`, you may want to create your own custom base-branch by forking
+`master` and merging the required topic branches to it. You could then
+work on top of this base-branch.  But keep in mind that this base-branch
+would only be known privately to you.  So when you are ready to send
+your patches to the list, be sure to communicate how you created it in
+your cover letter.  This critical piece of information would allow
+others to recreate your base-branch on their end in order for them to
+try out your work.
 
 Finally, note that some parts of the system have dedicated maintainers
 with their own separate source code repositories (see the section