diff mbox series

[bug#63459] doc: Rewrite the branching strategy.

Message ID f339d15842370b97558b704593848e318462b68d.1683878120.git.mail@cbaines.net
State New
Headers show
Series [bug#63459] doc: Rewrite the branching strategy. | expand

Commit Message

Christopher Baines May 12, 2023, 7:55 a.m. UTC
Move away from using staging and core-updates, and make the strategy
independant of branch names.

Keep the 300 dependent threshold for changes to master, as I don't have any
specific reason to change this.

Most importantly, require using guix-patches issues to coordinate merging of
the branches, as I think that'll address the key issues that have shown up
recently where it's been unclear which branch should be merged next.

* doc/contributing.texi (Submitting Patches): Rewrite branching strategy.
---
 doc/contributing.texi | 58 +++++++++++++++++--------------------------
 1 file changed, 23 insertions(+), 35 deletions(-)


base-commit: 9d05f9a9f538061e1fdc5aedb0748d260fcf20f7
prerequisite-patch-id: ae24e25c683be86ce0b3fa1fde1bd30e3e08e248

Comments

Christopher Baines May 12, 2023, 8:04 a.m. UTC | #1
Christopher Baines <mail@cbaines.net> writes:

> Move away from using staging and core-updates, and make the strategy
> independant of branch names.
>
> Keep the 300 dependent threshold for changes to master, as I don't have any
> specific reason to change this.
>
> Most importantly, require using guix-patches issues to coordinate merging of
> the branches, as I think that'll address the key issues that have shown up
> recently where it's been unclear which branch should be merged next.
>
> * doc/contributing.texi (Submitting Patches): Rewrite branching strategy.
> ---
>  doc/contributing.texi | 58 +++++++++++++++++--------------------------
>  1 file changed, 23 insertions(+), 35 deletions(-)

Following on from the discussion recently about moving away from staging
and core-updates, I've sent a patch to update the branching strategy.

The key thing is obviously to just remove mentions of staging and
core-updates, making the guidance more generic.

However, I've also added some requirements to use guix-patches issues to
track the intentions to merge branches, as I think that'll help address
some of the issues that came up recently with uncertainty around which
branch will be merged next.

I'm also hoping that these issues then can be used to automate the QA
process, triggering the qa-frontpage to automatically start building the
relevant branches.

Thanks,

Chris
Ludovic Courtès May 19, 2023, 1:22 p.m. UTC | #2
Hello,

Thanks for this initiative!

Christopher Baines <mail@cbaines.net> skribis:

> Move away from using staging and core-updates, and make the strategy
> independant of branch names.
>
> Keep the 300 dependent threshold for changes to master, as I don't have any
> specific reason to change this.
>
> Most importantly, require using guix-patches issues to coordinate merging of
> the branches, as I think that'll address the key issues that have shown up
> recently where it's been unclear which branch should be merged next.
>
> * doc/contributing.texi (Submitting Patches): Rewrite branching strategy.

[...]

> +Changes to packages with 300 dependent packages or less can be pushed to
> +the @code{master} branch.
> +
> +Larger changes should be first pushed to a branch other than
> +@code{master}. This allows for testing and for the build farms to
> +process the changes prior to being pushed to the @code{master} branch.

I’d be more specific:

  Larger changes should first be pushed to a topic branch other than
  @code{master}; the set of changes should be consistent---e.g., ``GNOME
  update'', ``NumPy update'', etc.  This allows for testing: the branch
  will automatically show up at
  @indicateurl{https://qa.guix.gnu.org/branch/@var{branch}}, with an
  indication of its build status on various platforms.

“Automatic” is a bit of an overstatement; that sentence probably needs
to be tweaked.  :-)  But I think it’s good to link to the QA platform to
make things more concrete.

> +To help coordinate the merging of branches, you must create a new
> +guix-patches issue each time you wish to merge a branch. These issues
                                                          ^
+ (@pxref{Tracking Bugs and Patches})

> +indicate the order in which the branches should be merged, so take a
> +look at the open issues for merging branches and mark the issue you
> +create as blocked by the issue previously at the back of the queue.

s/blocked/@dfn{blocked}/

Perhaps add a footnote or paren stating how to “block” an issue in
Debbugs?

> +Normally branches will be merged in a ``first come, first merged''
> +manor, tracked through the guix-patches issues. If you agree a different

s/manor/manner/
s/agree a/agree on a/

> +order with those involved, you can track this by updating which issues
> +block which other issues. Therefore, to know which branch is at the
> +front of the queue, look for the issue which isn't blocked by any other
> +branch merges.
> +
> +Once a branch is at the front of the queue, wait until sufficient time
> +has passed for the build farms to have processed the changes, and for
> +the necessary testing to have happened.

This is a bit technical.  How can I know “which branch is at the front
of the queue”?  Even as a seasoned Debbugs users, I’m not sure what I’m
supposed to do here.  Do you think we could provide ready to use
commands (debbugs.el or ‘mumi’) or at least a sequence of steps to
follow?

Last but not least: two spaces after end-of-sentence period please.  :-)

This is mostly about tweaking words; I think this is a great step
forward, very much in line with what was discussed in February at the
Guix Days.  Thank you!

Ludo’.
Christopher Baines May 23, 2023, 5:28 p.m. UTC | #3
Ludovic Courtès <ludo@gnu.org> writes:

> Hello,
>
> Thanks for this initiative!
>
> Christopher Baines <mail@cbaines.net> skribis:
>
>> Move away from using staging and core-updates, and make the strategy
>> independant of branch names.
>>
>> Keep the 300 dependent threshold for changes to master, as I don't have any
>> specific reason to change this.
>>
>> Most importantly, require using guix-patches issues to coordinate merging of
>> the branches, as I think that'll address the key issues that have shown up
>> recently where it's been unclear which branch should be merged next.
>>
>> * doc/contributing.texi (Submitting Patches): Rewrite branching strategy.
>
> [...]
>
>> +Changes to packages with 300 dependent packages or less can be pushed to
>> +the @code{master} branch.
>> +
>> +Larger changes should be first pushed to a branch other than
>> +@code{master}. This allows for testing and for the build farms to
>> +process the changes prior to being pushed to the @code{master} branch.
>
> I’d be more specific:
>
>   Larger changes should first be pushed to a topic branch other than
>   @code{master}; the set of changes should be consistent---e.g., ``GNOME
>   update'', ``NumPy update'', etc.  This allows for testing: the branch
>   will automatically show up at
>   @indicateurl{https://qa.guix.gnu.org/branch/@var{branch}}, with an
>   indication of its build status on various platforms.
>
> “Automatic” is a bit of an overstatement; that sentence probably needs
> to be tweaked.  :-)  But I think it’s good to link to the QA platform to
> make things more concrete.

That sounds fine to me. Everything apart from starting the builds is
already automatic, and I want to automate that through the issues
described here.

>> +To help coordinate the merging of branches, you must create a new
>> +guix-patches issue each time you wish to merge a branch. These issues
>                                                           ^
> + (@pxref{Tracking Bugs and Patches})
>
>> +indicate the order in which the branches should be merged, so take a
>> +look at the open issues for merging branches and mark the issue you
>> +create as blocked by the issue previously at the back of the queue.
>
> s/blocked/@dfn{blocked}/
>
> Perhaps add a footnote or paren stating how to “block” an issue in
> Debbugs?

Yeah, I'll try and write something.

>> +Normally branches will be merged in a ``first come, first merged''
>> +manor, tracked through the guix-patches issues. If you agree a different
>
> s/manor/manner/
> s/agree a/agree on a/
>
>> +order with those involved, you can track this by updating which issues
>> +block which other issues. Therefore, to know which branch is at the
>> +front of the queue, look for the issue which isn't blocked by any other
>> +branch merges.
>> +
>> +Once a branch is at the front of the queue, wait until sufficient time
>> +has passed for the build farms to have processed the changes, and for
>> +the necessary testing to have happened.
>
> This is a bit technical.  How can I know “which branch is at the front
> of the queue”?  Even as a seasoned Debbugs users, I’m not sure what I’m
> supposed to do here.  Do you think we could provide ready to use
> commands (debbugs.el or ‘mumi’) or at least a sequence of steps to
> follow?

So, I think there's two technical hurdles to overcome here. The first is
identifying the issues for merging branches, maybe for that we can set
out a format for the title of the bug, but I'm very open to
suggestions. Any way of identifying the open issues should be usable
through debbugs.el and mumi.

The second hurdle is the queuing behaviour, which I think the blocking
behaviour is a natural fit for. Maybe the tooling is lacking but I think
that can be addressed.

I want the qa-frontpage to display the queue of branches (and issues) in
a clear way, as well as providing links to make changes (as it does for
marking issues as moreinfo).

> Last but not least: two spaces after end-of-sentence period please.  :-)
>
> This is mostly about tweaking words; I think this is a great step
> forward, very much in line with what was discussed in February at the
> Guix Days.  Thank you!

Great, thanks for taking a look!

Chris
Christopher Baines May 31, 2023, 9:46 a.m. UTC | #4
Christopher Baines <mail@cbaines.net> writes:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Hello,
>>
>> Thanks for this initiative!
>>
>> Christopher Baines <mail@cbaines.net> skribis:
>>
>>> +order with those involved, you can track this by updating which issues
>>> +block which other issues. Therefore, to know which branch is at the
>>> +front of the queue, look for the issue which isn't blocked by any other
>>> +branch merges.
>>> +
>>> +Once a branch is at the front of the queue, wait until sufficient time
>>> +has passed for the build farms to have processed the changes, and for
>>> +the necessary testing to have happened.
>>
>> This is a bit technical.  How can I know “which branch is at the front
>> of the queue”?  Even as a seasoned Debbugs users, I’m not sure what I’m
>> supposed to do here.  Do you think we could provide ready to use
>> commands (debbugs.el or ‘mumi’) or at least a sequence of steps to
>> follow?
>
> So, I think there's two technical hurdles to overcome here. The first is
> identifying the issues for merging branches, maybe for that we can set
> out a format for the title of the bug, but I'm very open to
> suggestions. Any way of identifying the open issues should be usable
> through debbugs.el and mumi.
>
> The second hurdle is the queuing behaviour, which I think the blocking
> behaviour is a natural fit for. Maybe the tooling is lacking but I think
> that can be addressed.
>
> I want the qa-frontpage to display the queue of branches (and issues) in
> a clear way, as well as providing links to make changes (as it does for
> marking issues as moreinfo).

I've sent a v2 now which makes more changes, most importantly it pulls
the content out from the "Submitting Patches" section to it's own
section, and also moves content from the Commit Policy in and references
it.

I've also made some progress with the qa-frontpage, it now shows a list
of branches with the corresponding issues on the homepage.
Christopher Baines June 8, 2023, 2:24 p.m. UTC | #5
Hey!

The changes in #63459 have strayed now in to touching the commit policy
[1]. My intent was to simplify the guidance by grouping it better, but I
think the significant change here is that the commit policy now
references the entire branching strategy, rather than just talking about
sending patches for review.

1: https://guix.gnu.org/manual/devel/en/html_node/Commit-Access.html#Commit-Policy

That new branching strategy makes some "should" requirements on sending
patches for review and pushing to topic branches for larger changes. It
also makes a "must" requirement on opening guix-patches issues to track
and manage merging branches.

I'd like to merge these changes next week since they've been up for a
few weeks, so do comment if you have any thoughts or if you'd like more
time to review them.

Thanks,

Chris
Andreas Enge June 9, 2023, 10:10 a.m. UTC | #6
Hello Chris,

thanks for taking up this issue! I agreed with Ludovic's comments, so
things look good now for me. A very minor point: In the section on
"trivial" changes, I would drop this sentence (which was already there
before):
"This is subject to being adjusted, allowing individuals to commit directly
on non-controversial changes on parts they’re familiar with."
The sentence is meaningless, as everything is all the time subject to being
adjusted; and we do not have immediate plans to adjust it.

Looking forward to the merge since it clarifies things and removes the
staging and core-updates branches not only from our minds, but also the
texts.

Andreas
Christopher Baines June 11, 2023, 9:37 a.m. UTC | #7
Andreas Enge <andreas@enge.fr> writes:

> thanks for taking up this issue! I agreed with Ludovic's comments, so
> things look good now for me. A very minor point: In the section on
> "trivial" changes, I would drop this sentence (which was already there
> before):
> "This is subject to being adjusted, allowing individuals to commit directly
> on non-controversial changes on parts they’re familiar with."
> The sentence is meaningless, as everything is all the time subject to being
> adjusted; and we do not have immediate plans to adjust it.

My reading of this line is that "adjusted" is probably not the right
word to use, but I think the intent here is to talk about how currently
it's accepted that people can and will push non-controversial changes on
parts they’re familiar with directly to master.

I'm not sure if others read this similarly.
Andreas Enge June 11, 2023, 9:53 a.m. UTC | #8
Am Sun, Jun 11, 2023 at 10:37:14AM +0100 schrieb Christopher Baines:
> My reading of this line is that "adjusted" is probably not the right
> word to use, but I think the intent here is to talk about how currently
> it's accepted that people can and will push non-controversial changes on
> parts they’re familiar with directly to master.

I read it the other way round: Right now it is not accepted, but it might
be adjusted to allow non-controversial changes in the future. Actually
the concept of "non-controversial commits" is probably controversial
in itself...

Andreas
Maxim Cournoyer June 12, 2023, 1:28 a.m. UTC | #9
Hi Christopher,

Christopher Baines <mail@cbaines.net> writes:

> Andreas Enge <andreas@enge.fr> writes:
>
>> thanks for taking up this issue! I agreed with Ludovic's comments, so
>> things look good now for me. A very minor point: In the section on
>> "trivial" changes, I would drop this sentence (which was already there
>> before):
>> "This is subject to being adjusted, allowing individuals to commit directly
>> on non-controversial changes on parts they’re familiar with."
>> The sentence is meaningless, as everything is all the time subject to being
>> adjusted; and we do not have immediate plans to adjust it.
>
> My reading of this line is that "adjusted" is probably not the right
> word to use, but I think the intent here is to talk about how currently
> it's accepted that people can and will push non-controversial changes on
> parts they’re familiar with directly to master.
>
> I'm not sure if others read this similarly.

That's how I read it as well.  I like the ability for people to, at
times depending on the situation, choose to push directly to fix or
update something instead of going through the otherwise recommended 1
week QA/review flow.
Christopher Baines June 12, 2023, 8:19 p.m. UTC | #10
I've now pushed this to master as
0ea096ae23fa81f05ce97e5e61c15647c0a475ec.

There's still lots to improve, both within the guidance and in addition
to it.

Top on my list is making some requirements about the issues to open when
you want to merge a branch (e.g. specifying the title format so that
qa.guix.gnu.org can detect them).

Thanks,

Chris
diff mbox series

Patch

diff --git a/doc/contributing.texi b/doc/contributing.texi
index 7bf350ee0d..c54910e34d 100644
--- a/doc/contributing.texi
+++ b/doc/contributing.texi
@@ -1264,41 +1264,29 @@  Submitting Patches
 @c See <https://lists.gnu.org/archive/html/guix-devel/2016-10/msg00933.html>.
 @cindex branching strategy
 @cindex rebuild scheduling strategy
-Depending on the number of dependent packages and thus the amount of
-rebuilding induced, commits go to different branches, along these lines:
-
-@table @asis
-@item 300 dependent packages or less
-@code{master} branch (non-disruptive changes).
-
-@item between 300 and 1,800 dependent packages
-@code{staging} branch (non-disruptive changes).  This branch is intended
-to be merged in @code{master} every 6 weeks or so.  Topical changes
-(e.g., an update of the GNOME stack) can instead go to a specific branch
-(say, @code{gnome-updates}).  This branch is not expected to be
-buildable or usable until late in its development process.
-
-@item more than 1,800 dependent packages
-@code{core-updates} branch (may include major and potentially disruptive
-changes).  This branch is intended to be merged in @code{master} every
-6 months or so.  This branch is not expected to be buildable or usable
-until late in its development process.
-@end table
-
-All these branches are @uref{https://@value{SUBSTITUTE-SERVER-1},
-tracked by our build farm} and merged into @code{master} once
-everything has been successfully built.  This allows us to fix issues
-before they hit users, and to reduce the window during which pre-built
-binaries are not available.
-
-When we decide to start building the @code{staging} or
-@code{core-updates} branches, they will be forked and renamed with the
-suffix @code{-frozen}, at which time only bug fixes may be pushed to the
-frozen branches.  The @code{core-updates} and @code{staging} branches
-will remain open to accept patches for the next cycle.  Please ask on
-the mailing list or IRC if unsure where to place a patch.
-@c TODO: It would be good with badges on the website that tracks these
-@c branches.  Or maybe even a status page.
+Changes to packages with 300 dependent packages or less can be pushed to
+the @code{master} branch.
+
+Larger changes should be first pushed to a branch other than
+@code{master}. This allows for testing and for the build farms to
+process the changes prior to being pushed to the @code{master} branch.
+
+To help coordinate the merging of branches, you must create a new
+guix-patches issue each time you wish to merge a branch. These issues
+indicate the order in which the branches should be merged, so take a
+look at the open issues for merging branches and mark the issue you
+create as blocked by the issue previously at the back of the queue.
+
+Normally branches will be merged in a ``first come, first merged''
+manor, tracked through the guix-patches issues. If you agree a different
+order with those involved, you can track this by updating which issues
+block which other issues. Therefore, to know which branch is at the
+front of the queue, look for the issue which isn't blocked by any other
+branch merges.
+
+Once a branch is at the front of the queue, wait until sufficient time
+has passed for the build farms to have processed the changes, and for
+the necessary testing to have happened.
 
 @item
 @cindex determinism, of build processes