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

[Ludovic Courtès]

Hi!

Christopher Baines <mail@cbaines.net> skribis:

> Add more examples of when it can be appropriate to push changes without
> review, as I think this can be appropriate in the case of trivial changes (as
> mentioned before), but also non-trivial fixes.
>
> No longer suggest pushing simple new packages or package upgrades (that don't
> cause lots of rebuilds) without sending to guix-patches. Now there's some
> automation for testing changes sent to guix-patches, sending changes there
> before pushing can mean that more rigerious testing takes place and help speed
> up substitutes becoming available. This is true, even if no human review takes
> place.
>
> Only suggest waiting one week for review for simpler changes, wait two weeks
> for more significant changes.
>
> Also, reorder some of the information in this section so it's grouped together
> better.
>
> * doc/contributing.texi (Commit Policy): Tweak.

FWIW I like the spirit of these changes.

Now to the letter…  :-)

>  @subsection Commit Policy
>  
> -If you get commit access, please make sure to follow
> -the policy below (discussions of the policy can take place on
> +If you get commit access, please make sure to follow the policy below
> +(discussions of the policy can take place on
>  @email{guix-devel@@gnu.org}).
>  
> -Non-trivial patches should always be posted to
> -@email{guix-patches@@gnu.org} (trivial patches include fixing typos,
> -etc.).  This mailing list fills the patch-tracking database
> -(@pxref{Tracking Bugs and Patches}).
> +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

Similar to zimoun’s first comment I think, I would like the beginning of
the sentence to clearly tell you whether it’s the situation you’re
interested in.  “For a minority of changes” doesn’t fit the bill in my
view.

So I would suggest something along the lines of:

  Changes should be posted to @email{guix-patches@@gnu.org}.  This
  mailing list […].  It also allows patches to be picked up and tested
  by the quality assurance robot; the result of that testing eventually
  shows up on the dashboard at
  @indicateurl{https://qa.guix.gnu.org/issue/@var{number}}, where
  @var{number} is the number assigned by the issue tracker.  Leave time
  […] it’s OK to commit.

  As an exception, some changes considered consensual and ``trivial'' or
  ``obvious'' may instead be pushed directly.  These include: fixing
  typos, and reverting commits that caused immediate problems.

That way we state the general rule first, and the exception next.  That
also explicitly mentions how that relates to qa.guix.

Regarding the list of exceptions, I feel that these two exceptions
listed here may be less than what we may except on a day-to-day basis;
perhaps there are other things to add there, but I’m not sure what.

Would be nice to get feedback from a maintainer too.

Thanks for working on this!

Ludo’.