diff mbox series

[4/5] SubmittingPatches: remove confusing guidance about base branches

Message ID 55bed55cb8859ac7b5b4f464232258f410b4d202.1688778359.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series SubmittingPatches: clarify which branch to use | expand

Commit Message

Linus Arver July 8, 2023, 1:05 a.m. UTC
From: Linus Arver <linusa@google.com>

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
"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 underlying principle of this guidance was probably something like
"base your work on the earlier-in-history branch so your change can be
merged forward". However, this principle is already concretely explained
in the accompanying bullet points. This principle should only come into
play if none of the scenarios described in the bullet points apply ---
and such a situation would be exceedingly rare.

Also, the guidance's wording of using the "oldest" branch is confusing
when read together with the rest of this section, because three of the
four named branches discussed ("master", "next", and "seen") move
frequently enough to not be considered "old" at all.

For these reasons, remove the guidance _without_ preserving the meaning
of the underlying principle, and instead add an overview of the four
named branches.

Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Linus Arver <linusa@google.com>
---
 Documentation/SubmittingPatches | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

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

> For these reasons, remove the guidance _without_ preserving the meaning
> of the underlying principle, and instead add an overview of the four
> named branches.

Meaning that this rewrites the guidance and changes the meaning of
the underlying principle?

> -In general, always base your work on the oldest branch that your
> -change is relevant to.
> +The following branches are the typical starting points for new work:
> +
> +* maint
> +* master
> +* next
> +* seen
> +
> +These branches are explained in detail in linkgit:gitworkflows[7].
> +Choose the appropriate branch depending on the following scenarios:

Please never suggest to build anything on 'next' or 'seen'.  They
are inappropriate to base your work on, if your topic wants to have
a realistic chance to graduate to 'master'.

If you are making tree-wide changes, while somebody else is also
making another tree-wide changes, your topic may have severe overlap
with the other person's topic.  In which case, you may be tempted to
build on 'next' that has the other person's topic, but doing so would
mean you'll not just depend on the other topic, but with all the
other topics that are already in 'next'.

That would make the basic choices simpler.

 * If you are fixing bugs in the released version, build on 'maint'
   (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).

 * If you are adding new features, build on 'master'.

Under exceptional circumstances that you need to depend on a
selected few topics that are already in 'next' but not in 'master',
you may want to fork your base-branch from 'master', merge these
selected few topics to it, and call that your base-branch (which
nobody else has).  And then you build on top of it.  When sending
patches out, because your synthetic base-branch is something only
you have, you'd need to communicate how you created it in your cover
letter to allow others to recreate it.

Thanks.
Linus Arver July 13, 2023, 9:54 p.m. UTC | #2
Junio C Hamano <gitster@pobox.com> writes:

> "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> For these reasons, remove the guidance _without_ preserving the meaning
>> of the underlying principle, and instead add an overview of the four
>> named branches.
>
> Meaning that this rewrites the guidance

Yes.

> and changes the meaning of the underlying principle?

Hmm, no. I think I should have written in my commit message instead:

--8<---------------cut here---------------start------------->8---
For these reasons, remove the guidance while still preserving the
meaning of the underlying principle by adding an overview of the four
named branches.
--8<---------------cut here---------------end--------------->8---

However, I now think deleting the "base your work on the oldest branch
that your change is relevant to" text was unnecessarily harsh. I think I
can reword it to make it sound less contrary to the accompanying bullet
points.

Will update in v2.

>> -In general, always base your work on the oldest branch that your
>> -change is relevant to.
>> +The following branches are the typical starting points for new work:
>> +
>> +* maint
>> +* master
>> +* next
>> +* seen
>> +
>> +These branches are explained in detail in linkgit:gitworkflows[7].
>> +Choose the appropriate branch depending on the following scenarios:
>
> Please never suggest to build anything on 'next' or 'seen'.  They
> are inappropriate to base your work on, if your topic wants to have
> a realistic chance to graduate to 'master'.

I only included "next" and "seen" here just below "maint" and "master"
because they were included as OK-places to start new work (albeit in
exceptional cases) in one of the bullet points:

--8<---------------cut here---------------start------------->8---
* 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).
--8<---------------cut here---------------end--------------->8---

> If you are making tree-wide changes, while somebody else is also
> making another tree-wide changes, your topic may have severe overlap
> with the other person's topic.  In which case, you may be tempted to
> build on 'next' that has the other person's topic, but doing so would
> mean you'll not just depend on the other topic, but with all the
> other topics that are already in 'next'.

Good point. I will include this tip in v2 (seems like
something that would be especially helpful for newer contributors).

> That would make the basic choices simpler.
>
>  * If you are fixing bugs in the released version, build on 'maint'
>    (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).
>
>  * If you are adding new features, build on 'master'.
>
> Under exceptional circumstances that you need to depend on a
> selected few topics that are already in 'next' but not in 'master',
> you may want to fork your base-branch from 'master', merge these
> selected few topics to it, and call that your base-branch (which
> nobody else has).  And then you build on top of it.  When sending
> patches out, because your synthetic base-branch is something only
> you have, you'd need to communicate how you created it in your cover
> letter to allow others to recreate it.

I strongly agree that this is simpler. One thing I would change is to
use a phrase like "start your work" instead of the word "build" because
the latter on quick glance could be misinterpreted as literally building
(compiling/packaging) the project.

Will incorporate in v2 (thank you for the suggestion; will credit you in
a "Helped-by: ..." trailer).
diff mbox series

Patch

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 48918181f49..ef39808f568 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -10,8 +10,15 @@  available which covers many of these same guidelines.
 [[base-branch]]
 === Decide which branch to base your work on.
 
-In general, always base your work on the oldest branch that your
-change is relevant to.
+The following branches are the typical starting points for new work:
+
+* maint
+* master
+* next
+* seen
+
+These branches are explained in detail in linkgit:gitworkflows[7].
+Choose the appropriate branch depending on the following scenarios:
 
 * 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