[bug#59513] [PATCH] doc: contributing: Tweak the Commit Policy.

[Christopher Baines]

zimoun <zimon.toutoune@gmail.com> writes:

> Hi Chris,

Hey! Thanks for taking a look.

> On Wed, 23 Nov 2022 at 10:49, Christopher Baines <mail@cbaines.net> wrote:
>
>> +For a minority of changes, it can be appropriate to push them directly
>> +without sending them for review.  This includes both trivial changes
>> +(e.g. fixing typos) but also reverting problomatic changes and
>                                               -^
>> +addressing regressions.
>
>
>> -For patches that just add a new package, and a simple one, it's OK to
>> -commit, if you're confident (which means you successfully built it in a
>> -chroot setup, and have done a reasonable copyright and license
>> -auditing).  Likewise for package upgrades, except upgrades that trigger
>> -a lot of rebuilds (for example, upgrading GnuTLS or GLib).  We have a
>> -mailing list for commit notifications (@email{guix-commits@@gnu.org}),
>> -so people can notice.  Before pushing your changes, make sure to run
>> -@code{git pull --rebase}.
>> +In general though, all changes should be posted to
>> +@email{guix-patches@@gnu.org}.  This mailing list fills the
>> +patch-tracking database (@pxref{Tracking Bugs and Patches}).  Leave time
>> +for a review, without committing anything (@pxref{Submitting Patches}).
>> +If you didn’t receive any reply after one week (two weeks for more
>> +significant changes), and if you're confident, it's OK to commit.
>
> I would write:
>
>         … changes), and if you're confident (which means you
>         successfully built it in a chroot setup, and have done a
>         reasonable copyright and license auditing), it’s OK to commit.

chroot setup doesn't really make sense to me, I'm not sure why that
needs specifying (like do we not want things for the Hurd pushing, since
the guix-daemon doesn't support build isolation there yet)?

Also, this guidance is very general, and I think it should be applicable
to all changes. We already trust people with commit access to know what
needs doing, I see this documentation as more about how, so I'd prefer
not to try and put a list here.

> and I would keep the «two weeks» instead of the «one week except».

My reason for changing this is that I think waiting two weeks after
sending a simple patch is unreasonable. The value from the automated
testing will come after one to two days, I just put a week to avoid
changing it too much, but maybe the lower bound should be two days.

> I think it is also useful to provide the information about commit
> notifications (guix-commits mailing list).

Why though? What do we expect people with commit access to do when they
read about that here?

> For what it is worth, I find clearer the structure,
>
>     For patches that …
>     For anything else, …
>
> or
>
>     For a minority of changes, …
>     For anything else, …
>
> than «In general though, all changes …».

That seems fine to me, I think "everything" maybe carries more weight
than "anything" though.

Thanks,

Chris

signature.asc
Description: PGP signature