Subject: Re: [PATCH v2] doc: contributing: Tweak the Commit Policy.

From: Liliana Marie Prikler <[email-masked]>
Date: Mon, 12 Dec 2022 21:27:45 +0100

Am Montag, dem 12.12.2022 um 10:49 +0000 schrieb Christopher Baines:
> 
> Liliana Marie Prikler <[email-masked]> writes:
> 
> > Am Donnerstag, dem 08.12.2022 um 11:20 +0000 schrieb Christopher
> > Baines:
> > > 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
> > 
> > rigorous
> 
> Thanks, I've fixed that locally now.
> 
> > > 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.
> > > ---
> > >  doc/contributing.texi | 41 ++++++++++++++++++-------------------
> > > ----
> > >  1 file changed, 18 insertions(+), 23 deletions(-)
> > > 
> > > diff --git a/doc/contributing.texi b/doc/contributing.texi
> > > index 6a8ffd6524..d2e7abba98 100644
> > > --- a/doc/contributing.texi
> > > +++ b/doc/contributing.texi
> > > @@ -1824,23 +1824,26 @@ It additionally calls @code{make check-
> > > channel-news} to be sure
> > > 
> > >  @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 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}.
> > > +Changes should be posted to @email{guix-patches@@gnu.org}.  This
> > > mailing
> > > +list fills the patch-tracking database (@pxref{Tracking Bugs and
> > > +Patches}).  It also allows patches to be picked up and tested by
> > > the
> > > +quality assurance tooling; 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
> > > +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 reword that so
> >   (not significant ∧ confident ∧ qa_green) → good after 1 week
> > whereas
> >   (not significant ∧ confident ∧ qa_unknown) → good after 2 weeks
> > and significant changes should anyway take 2 weeks.
> 
> While I like the intent here, for the moment I prefer a simpler
> policy. Maybe we can move in this direction when the QA tooling is
> more usable and reliable.
I wrote this partly with the intent of resolving an ambiguity, but I
agree on the principle of having a simple policy.  I take it that QA
status is not that important at the moment – more that QA knows about
the patch and has already started building packages?

Cheers