From: "Ludovic Courtès" <ludo@gnu.org>
To: Christopher Baines <mail@cbaines.net>
Cc: guix-devel@gnu.org
Subject: Re: More thoughts on Patchwork and Guix patch review/quality assurance
Date: Sat, 05 Dec 2020 16:04:55 +0100 [thread overview]
Message-ID: <87tut05m5k.fsf@gnu.org> (raw)
In-Reply-To: <87v9dos547.fsf@cbaines.net> (Christopher Baines's message of "Sun, 29 Nov 2020 12:46:16 +0000")
Hi Chris!
Christopher Baines <mail@cbaines.net> skribis:
> If you're interested in using this to review patches, the place to start
> is Patchwork, so visit:
>
> https://patchwork.cbaines.net/
>
> If you want an account, please email me. Unfortunately I had to disable
> registration due to spam.
>
> When you click through to a patch, the thing to look at is the
> checks. These should link you to the relevant bug, Git branch and Guix
> Data Service comparison. For the Guix Data Service comparison, the
> things to note are the lint warnings and then clicking through to the
> "Compare package derivations" page.
Very nice! This is useful info when reviewing, and it’s much harder to
get this at a glance locally.
> ### Use Patchwork, rather than Debbugs to track patches
>
> So this has been a thing for 4 years now (see [2] for some graphs). I
> think I was there in the bar in Brussels when it was discussed.
>
> 2: https://debbugs.gnu.org/rrd/guix-patches.html
I remember that night in Brussels, it’s been a while! :-)
> The main disadvantage I see of using Debbugs is that you have to get a
> bug number before you can Git send-email, in the case where you're
> sending multiple patches.
>
> The main advantage I get from Debbugs is that searching via the bug
> number is really useful.
>
> I'm unsure, I think Debbugs was introduced to help keep track of
> patches, and avoid them getting forgotten in guix-devel. Patchwork will
> do this too, but maybe Debbugs is providing more value than the cost of
> multi-patch series being slightly more difficult to submit?
To me, the main advantages of Debbugs are:
1. the Emacs interface (I use it a lot as a reviewer, no need to open
a browser);
2. the clean web interface at <https://issues.guix.gnu.org>, which I
find less intimidating, less cluttered, and hopefully more familiar
than that of Patchwork; it also has a useful search interface now;
3. Having numbers and short URLs to reference issues.
Now, Patchwork “checks” and how you use them are great; simplifying
multi-patch submissions is great too.
If Mumi had hooks that could be used to run and display those checks,
I’d be super happy. But I don’t know whether it’s reasonable, nor
whether that’s something others would also like. :-)
All this could be complemented by CLIs to “the official Guix review
services” (Patchwork, Mumi, Data Service, etc.). You could run:
guix review 1234
and it’d fetch the patch(es), apply them, and/or display a status
summary based on data published by Patchwork & co. It’s probably not
that much work if those services have HTTP APIs, and it could facilitate
adoption.
> ### Scheduled and regular collaboration on IRC to review patches
>
> When I can make some time, I'll try this out, but if anyone has some
> time they can set aside, please go ahead!
Yes! A weekly meeting where committers and submitters are around and
focusing on getting patches merged would be great!
> ### Help users with the submitting part of submitting patches
>
> Using git send-email works well with Patchwork at least, and it's fine
> with Debbugs if you've got a single patch, but a little more time
> consuming if you're got more than one patch.
>
> Attaching a single patch file to an email works OK I think, however
> attaching multiple patch files to the same email confuses git am and
> Patchwork I believe.
>
> I think other approaches like copying the patch in to the body of an
> email, or just copying the output of git diff in to an email are very
> brittle and can make it more time consuming to try and recover or
> recreate the commit(s).
>
> Having the emails for patches is useful for reviewing, but maybe there's
> a way users could push a branch somewhere and then have some service do
> the git send-email thing on their behalf?
Yes, that would be nice. To me that’s less important than getting the
checks you implemented actually used, though.
> ### Notify interested people about patches
>
> This is something I've been thinking about more generally, supporting
> email subscriptions for things like a new version of a package being
> available, or the package failing to build on
> master/staging/core-updates.
>
> But specifically for patches, maybe there could be a way of subscribing
> so that you're emailed when a patch series upgrades a package you're
> subscribed to, or breaks a package you're subscribed to.
>
> This could help get more people involved in reviewing patches, by making
> it easier for the interested people to find out about patches they're
> interested in.
+1!
> ### Helping people who aren't committers review patches
>
> I don't know how much of this happens, but it might be something to
> better support?
>
> Better supporting it could mean documenting how to get things that have
> passed review merged in, like emailing a list of committers when a patch
> series is ready to merge.
Agreed. Non-committers do review occasionally and I find it very useful.
> ### Making automated testing more rigorous
>
> So, providing the patches can be automatically processed and apply
> successfully, I'm currently building packages for x86_64-linux and
> i686-linux.
>
> There are more things that could be built if I amend the relevant
> script:
>
> - System tests (maybe just x86_64-linux is relevant?
>
> - Cross-compiled packages (the Guix Data Service currently only
> generates these for x86_64-linux)
ci.guix.gnu.org is doing that and (gnu ci) as well as
etc/system-tests.scm and etc/release-manifest.scm formalize it.
> There are also some things that would require more work/hardware:
>
> - Building packages for architectures other than
> x86_64-linux/i668-linux.
>
> - I have some ideas on getting the Guix Build Coordinator agent
> running in a childhurd, some general substitute availability would
> also be good for getting this to work
Note that GNU/Hurd currently uses out-of-chroot builds; IMO that part
needs to be addressed (see <https://issues.guix.gnu.org/43857>).
> - Building packages on a range of hardware with a range of
> resources/configurations. This is kind of a better version of just
> building a package multiple times. It would help spot odd failures
> earlier, and give valuable data about the reproducibility of the
> build outputs.
>
> - Single core vs many cores
>
> - Not much RAM vs plenty of RAM
>
> - Different filesystems (btrfs, ext4, ...)
>
> - Different Linux-libre versions
>
> - Different system times (2025, 2101, ...)
>
> - Maybe building on other OS's (Debian, ...)
>
> These things require hardware availability and/or being able to
> control resources for a single build through the guix-daemon. It also
> requires a way in the Guix Build Coordinator to specify that you want
> builds to happen in this way, so build a single derivation many
> times, in each of these environments.
Some of these tests could be done by offloading to “childguix” VMs or by
creating derviations that build derivations in VM.
Thanks!
Ludo’.
prev parent reply other threads:[~2020-12-05 15:05 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-29 12:46 More thoughts on Patchwork and Guix patch review/quality assurance Christopher Baines
2020-12-05 15:04 ` Ludovic Courtès [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://guix.gnu.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87tut05m5k.fsf@gnu.org \
--to=ludo@gnu.org \
--cc=guix-devel@gnu.org \
--cc=mail@cbaines.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).