Changes to the branching/commit policy

Changes to the branching/commit policy

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 826 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

[bug#63459] Changes to the branching/commit policy

[Andreas Enge]

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

Re: Changes to the branching/commit policy

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 877 bytes --]


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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

Re: Changes to the branching/commit policy

[Andreas Enge]

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

[bug#63459] Changes to the branching/commit policy

[Maxim Cournoyer]

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.

-- 
Thanks,
Maxim

Re: Changes to the branching/commit policy

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 1306 bytes --]


Christopher Baines <mail@cbaines.net> writes:

> 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.

I've now merged these changes as
0ea096ae23fa81f05ce97e5e61c15647c0a475ec.

You can now see the updated Commit Policy on the website [1] (you might
need to force a refresh), as well as the new section on managing patches
and branches [2].

1: https://guix.gnu.org/manual/devel/en/html_node/Commit-Access.html#Commit-Policy
2: https://guix.gnu.org/manual/devel/en/html_node/Managing-Patches-and-Branches.html

Thanks,

Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

Re: Changes to the branching/commit policy

[John Kehayias]

Hi Chris,

On Mon, Jun 12, 2023 at 09:23 PM, Christopher Baines wrote:
>
> Christopher Baines <mail@cbaines.net> writes:
>
>> 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.
>
> I've now merged these changes as
> 0ea096ae23fa81f05ce97e5e61c15647c0a475ec.
>
> You can now see the updated Commit Policy on the website [1] (you might
> need to force a refresh), as well as the new section on managing patches
> and branches [2].
>
> 1: <https://guix.gnu.org/manual/devel/en/html_node/Commit-Access.html#Commit-Policy>
> 2: <https://guix.gnu.org/manual/devel/en/html_node/Managing-Patches-and-Branches.html>
>

Thanks for these changes! Question on branches (sorry if this was
covered in a previous thread, but now that we have new language in the
manual I figure this is a good place): do we have a convention on
branch names and subject headers for emailing patches for the branch?
e.g. does [PATCH <branch> 1/3] do anything on the QA end? Or does the
section about branch building for once patches are pushed to a branch
on Savannah? Does that mean pushing to a branch should follow the same
1-2 week review allowing QA builds? I guess patch series are always
built together on QA but wondering if there is anything else to be
aware of or needs mentioning to keep things tidy and clear.

Thanks!
John

Re: Changes to the branching/commit policy

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 2785 bytes --]


John Kehayias <john.kehayias@protonmail.com> writes:

>> I've now merged these changes as
>> 0ea096ae23fa81f05ce97e5e61c15647c0a475ec.
>>
>> You can now see the updated Commit Policy on the website [1] (you might
>> need to force a refresh), as well as the new section on managing patches
>> and branches [2].
>>
>> 1: <https://guix.gnu.org/manual/devel/en/html_node/Commit-Access.html#Commit-Policy>
>> 2: <https://guix.gnu.org/manual/devel/en/html_node/Managing-Patches-and-Branches.html>
>>
>
> Thanks for these changes! Question on branches (sorry if this was
> covered in a previous thread, but now that we have new language in the
> manual I figure this is a good place): do we have a convention on
> branch names and subject headers for emailing patches for the branch?
> e.g. does [PATCH <branch> 1/3] do anything on the QA end?

On the names of branches, there's some typical names like XXXX-team, or
wip-XXXX but really it's up to you.

QA doesn't currently support applying patches to anything other than the
master branch, but that's something that should probably be addressed at
some point. I'd encourage you to do whatever you think is useful here,
then hopefully QA can use that to act appropriately.

> Or does the section about branch building for once patches are pushed
> to a branch on Savannah?

I'm not sure what you're asking here.

> Does that mean pushing to a branch should follow the same 1-2 week
> review allowing QA builds? I guess patch series are always built
> together on QA but wondering if there is anything else to be aware of
> or needs mentioning to keep things tidy and clear.

Those durations mentioned in the commit policy [1] are intended to allow
for human review. For QA, it doesn't need to be time based as it can
report it's progress. Obviously it does need a bit of time to check
things, but I prefer to leave it up to people to assess the changes and
any information provided by QA and decide when it's appropriate to push.

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

On keeping things clear, I think with branches I'm hoping the issue
tracker can help with this, which is why there's now a strong
requirement to create a guix-patches issue when you want to merge a
branch, and use those issues to manage the timing.

For those issues, there's currently a convention of using the following
title: `Request for merging "XXXX" branch` e.g. [2]. That helps QA find
these issues and act accordingly, but of course if someone comes up with
a better way of naming these issues, we can just adjust the code in the
qa-frontpage.

2: https://issues.guix.gnu.org/63713
3: https://git.savannah.gnu.org/cgit/guix/qa-frontpage.git/tree/guix-qa-frontpage/branch.scm#n63

Thanks for your questions :)

Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

Re: Changes to the branching/commit policy

[John Kehayias]

Hi Chris,


On Sat, Jun 17, 2023 at 10:57 AM, Christopher Baines wrote:
>
> John Kehayias <john.kehayias@protonmail.com> writes:
>
>> Thanks for these changes! Question on branches (sorry if this was
>> covered in a previous thread, but now that we have new language in the
>> manual I figure this is a good place): do we have a convention on
>> branch names and subject headers for emailing patches for the branch?
>> e.g. does [PATCH <branch> 1/3] do anything on the QA end?
>
> On the names of branches, there's some typical names like XXXX-team, or
> wip-XXXX but really it's up to you.
>
> QA doesn't currently support applying patches to anything other than the
> master branch, but that's something that should probably be addressed at
> some point. I'd encourage you to do whatever you think is useful here,
> then hopefully QA can use that to act appropriately.
>

Sounds good, a simple identifier in the "[PATCH]" subject, like for
core-updates, seems simple enough to get started.

>> Or does the section about branch building for once patches are pushed
>> to a branch on Savannah?
>
> I'm not sure what you're asking here.

I'm confused myself over my wording, but I remember what I was trying
to get at:

Currently QA doesn't build patches if they cause a large number of
rebuilds correct? So for building a branch that requires this, will
that only happen on Cuirass with a new jobspec for the branch? Or will
this be addressed through changes to how QA builds? (Maybe this
answered below actually.)

>
>> Does that mean pushing to a branch should follow the same 1-2 week
>> review allowing QA builds? I guess patch series are always built
>> together on QA but wondering if there is anything else to be aware of
>> or needs mentioning to keep things tidy and clear.
>
> Those durations mentioned in the commit policy [1] are intended to allow
> for human review. For QA, it doesn't need to be time based as it can
> report it's progress. Obviously it does need a bit of time to check
> things, but I prefer to leave it up to people to assess the changes and
> any information provided by QA and decide when it's appropriate to push.
>
> 1: <https://guix.gnu.org/manual/devel/en/html_node/Commit-Access.html#Commit-Policy>
>

A separate issue which I wanted to get at was about pushing changes to
any branch on Savannah. Do we expect those to be at the same state as
anything that would go directly to master, just pending the testing of
builds basically? So, after the usual review period? Or can those be
more WIP and not polished yet? I suppose this is for a team/people
working on a branch to discuss and how it will then be merged to
master.

> On keeping things clear, I think with branches I'm hoping the issue
> tracker can help with this, which is why there's now a strong
> requirement to create a guix-patches issue when you want to merge a
> branch, and use those issues to manage the timing.
>
> For those issues, there's currently a convention of using the following
> title: `Request for merging "XXXX" branch` e.g. [2]. That helps QA find
> these issues and act accordingly, but of course if someone comes up with
> a better way of naming these issues, we can just adjust the code in the
> qa-frontpage.
>

I see, that's a nice way to then group up discussion if a branch has a
bunch of separate patch threads. Looks like a good idea!

So, to be concrete, with the mesa patch I just sent,
<https://issues.guix.gnu.org/64175> I can open a merge request issue for
QA to process the branch, once the branch is actually created on
Savannah? I assume with the pretty trivial version change here I could
do that to see how the builds go, but I'll hold off pending the thread
I just started about this branch/team.

> 2: <https://issues.guix.gnu.org/63713>
> 3:
> <https://git.savannah.gnu.org/cgit/guix/qa-frontpage.git/tree/guix-qa-frontpage/branch.scm#n63>
>
> Thanks for your questions :)
>
> Chris
>

And thanks again for all you do here, the Guix tooling has been making
great progress!

John

Re: Changes to the branching/commit policy

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 3210 bytes --]


John Kehayias <john.kehayias@protonmail.com> writes:

>>> Or does the section about branch building for once patches are pushed
>>> to a branch on Savannah?
>>
>> I'm not sure what you're asking here.
>
> I'm confused myself over my wording, but I remember what I was trying
> to get at:
>
> Currently QA doesn't build patches if they cause a large number of
> rebuilds correct? So for building a branch that requires this, will
> that only happen on Cuirass with a new jobspec for the branch? Or will
> this be addressed through changes to how QA builds? (Maybe this
> answered below actually.)

Yep, for issues there's a cap of 300 builds per system.

QA does build the branch at the front of the queue to be merged though,
currently that's ruby-team [1].

1: https://qa.guix.gnu.org/

>>> Does that mean pushing to a branch should follow the same 1-2 week
>>> review allowing QA builds? I guess patch series are always built
>>> together on QA but wondering if there is anything else to be aware of
>>> or needs mentioning to keep things tidy and clear.
>>
>> Those durations mentioned in the commit policy [1] are intended to allow
>> for human review. For QA, it doesn't need to be time based as it can
>> report it's progress. Obviously it does need a bit of time to check
>> things, but I prefer to leave it up to people to assess the changes and
>> any information provided by QA and decide when it's appropriate to push.
>>
>> 1: <https://guix.gnu.org/manual/devel/en/html_node/Commit-Access.html#Commit-Policy>
>>
>
> A separate issue which I wanted to get at was about pushing changes to
> any branch on Savannah. Do we expect those to be at the same state as
> anything that would go directly to master, just pending the testing of
> builds basically? So, after the usual review period? Or can those be
> more WIP and not polished yet? I suppose this is for a team/people
> working on a branch to discuss and how it will then be merged to
> master.

The latter. Non-master branches can be in whatever state is useful.

>> On keeping things clear, I think with branches I'm hoping the issue
>> tracker can help with this, which is why there's now a strong
>> requirement to create a guix-patches issue when you want to merge a
>> branch, and use those issues to manage the timing.
>>
>> For those issues, there's currently a convention of using the following
>> title: `Request for merging "XXXX" branch` e.g. [2]. That helps QA find
>> these issues and act accordingly, but of course if someone comes up with
>> a better way of naming these issues, we can just adjust the code in the
>> qa-frontpage.
>>
>
> I see, that's a nice way to then group up discussion if a branch has a
> bunch of separate patch threads. Looks like a good idea!
>
> So, to be concrete, with the mesa patch I just sent,
> <https://issues.guix.gnu.org/64175> I can open a merge request issue for
> QA to process the branch, once the branch is actually created on
> Savannah? I assume with the pretty trivial version change here I could
> do that to see how the builds go, but I'll hold off pending the thread
> I just started about this branch/team.

Currently QA just builds the branch at the front of the queue, but yes.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]