On commit access, patch review, and remaining healthy

On commit access, patch review, and remaining healthy

[Ludovic Courtès]

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

Hello Guix!

Following on the theme of patch review, I did some stats with the
attached tools on commits since 1.3.0:

  • 20,489 commits were made since then;

  • 4,476 were commits pushed on behalf of a non-committer;

  • of these, half were pushed by 2 committers, out of 40ish.

Some conclusions we can draw:

  • We have a strong core development team, which I think is great
    compared to many free software projects.

    Perhaps the flip side of this is that we make too little space to
    newcomers.  (I feel we’re almost the opposite of a typical
    Git{Hub,Lab}-hosted project where drive-by contributions are common
    and long-term commitment is rare.)

  • Review work is severely lacking.  The manual reads (info "(guix)
    Commit Access"):

      […] the project keeps moving forward because committers not only push
      their own awesome changes, but also offer some of their time
      _reviewing_ and pushing other people’s changes.  As a committer,
      you’re welcome to use your expertise and commit rights to help
      other contributors, too!

    Yet, most committers don’t allocate time to review and push other
    people’s changes.

    Why aren’t we, committers, not doing more review/apply work?  Is it
    too intimidating?  Would having a documented review checklist help?

    If you’re not using Emacs, what actionable steps should we take with
    mumi and other tools to help you (Arun made several proposals in
    their Guix Days talk)?  If you are using Emacs, does debbugs.el have
    shortcomings that make it a problem to review patches?

  • We need to be able to renew committers.  There’s a process in place
    to remove, at least temporarily, committers that have been inactive
    for a year or more, and I think it’s good (info "(guix) Commit
    Access").

    Maybe we should also encourage committers who have “moved on” to let
    the project know so we have a clearer picture of who’s in—meaning
    available not just to commit their own occasional patches, but also
    to help other contributors.

    In addition to that, we need to encourage contributors who are not
    committers yet, which obviously means reviewing and applying their
    contributions in a timely fashion.  We need to grow prolific
    contributors into leadership positions to they can become committers
    and take part into this whole process.

In short, we need to break out of a potentially vicious circle where
active members don’t make the work that would allow newcomers to get
more involved, at the risk of burning out themselves.

Let’s make sure this project keeps striving for decades to come!  :-)

Thoughts?

Ludo’.


[-- Attachment #2: the code --]
[-- Type: text/plain, Size: 2692 bytes --]

(use-modules (git)
             (git repository)
             (git reference)
             (git oid)
             (git tag)
             (git commit)
             (git structs)                        ;signature-email, etc.
             (guix git)
             (srfi srfi-1)
             (srfi srfi-26)
             (ice-9 match)
             (ice-9 vlist))

(define commit-author*
  (compose signature-name commit-author))
(define commit-committer*
  (compose signature-name commit-committer))

(define-syntax-rule (false-if-git-error exp)
  (catch 'git-error
    (lambda () exp)
    (const #f)))

(define* (fold-commits proc seed repo
                       #:key
                       (start (reference-target
                               (repository-head repo)))
                       end)
  "Call PROC on each commit of REPO, starting at START (an OID), and until
END if specified."
  (let loop ((commit (commit-lookup repo start))
             (result seed))
    (let ((parent (false-if-git-error (commit-parent commit))))
      (if parent
          (if (and end (oid=? (commit-id parent) end))
              (proc parent result)
              (loop parent (proc parent result)))
          result))))

(define (reviewers repo commits)
  "Return a list of review count/committer pairs."
  (define vhash
    (fold (lambda (commit result)
            (if (string=? (commit-author* commit)
                          (commit-committer* commit))
                result
                (vhash-cons (commit-committer* commit) #t
                            result)))
          vlist-null
          commits))

  (define committers
    (delete-duplicates
     (fold-commits (lambda (commit result)
                     (cons (commit-committer* commit)
                           result))
                   '()
                   repo)))

  (map (lambda (committer)
         (cons (vhash-fold* (lambda (_ count)
                              (+ 1 count))
                            0
                            committer
                            vhash)
               committer))
       committers))

(define (reviewer< r1 r2)
  (match r1
    ((count1 . name1)
     (match r2
       ((count2 . name2)
        (< count1 count2))))))

(define repo
  (repository-open "."))

(define commits
  (commit-difference (commit-lookup repo
                                    (reference-target (repository-head repo)))
                     (commit-lookup
                      repo
                      (tag-target-id
                       (tag-lookup
                        repo
                        (reference-target
                         (reference-lookup repo "refs/tags/v1.3.0")))))))

Re: On commit access, patch review, and remaining healthy

[Brian Cully via Development of GNU Guix and the GNU System distribution.]

Ludovic Courtès <ludo@gnu.org> writes:

>     If you are using Emacs, does debbugs.el have
>     shortcomings that make it a problem to review patches?

I’m new to debbugs in general, and Emacs’ debbugs mode in 
particular. However, I have been using it to track the status on 
some of my patches, or to just generally browse.

There are a few issues:

1) It’d be nice if ‘M-x debbug-guix’ existed. I (briefly) looked 
at making such a command, thinking it’d be relatively 
straight-forward to just copy ‘debbugs-gnu’ and tweak some 
variables, but when it looked like it was going to take more than 
10 minutes just to figure out how it was all put together I gave 
up as I can’t really dedicate time to it right now.

2) IMHO, the documentation for debbugs is pretty poor. It mostly 
relies on GNUS documentation for things like mode help; and while 
this makes sense, GNUS is a big, complex package (which is why I 
use mu4e instead of it), and it really raises the barrier for 
understanding debbugs quite a lot.

3) Even after reading all the documentation I could find, it 
doesn’t seem to work very well for an issue-tracker workflow. I 
still don’t know how to simply reply to a ticket. Let alone how to 
apply a patch to my tree (I did see documentation for how to do 
that to the Emacs source tree, but that’s not very useful — also, 
the key combinations feel very obtuse and hard for me to 
remember).

So, right now, I just use it to browse, since I find it easier 
than mumi, but everything else happens with external tools. It’s 
very possible my n00b is showing right now and it’s easier to work 
with than it seems at first blush, but I’d say that means the 
documentation needs some dedicated, non-GNUS-oriented love. It 
would be very helpful if there were a HOWTO, like we used to do in 
the old days, with how to use it for simple tasks, and stick it in 
the info documentation and on the web.

-bjc

Re: On commit access, patch review, and remaining healthy

[Pier-Hugues Pellerin]

Hello,

As a new-new Guix user, I did find the review process or the time 
it takes really long.
Maybe I've tackle too complex updates[0], I don't know but I don't 
have a clear path how to push it.

As a dev, I am not super used to the email-patches workflow, I am 
more used to the pull-request
git{hub|lab} process and do work on large open source project and 
reviewing code is a tedious but necessary task.

Also, I don't want to say that one workflow is superior to the 
other but one thing that shines with theses forge
is the automation and the simplicity to add tooling on pull 
request to reduce the burden to the reviewer:

- linting
- checks
- commits log formatting check
- CI jobs
- codeowner assignment
- build packages (partial or complete rebuild the world if needed)
- license check
- notification for stable pull request.

All theses are useful quality of life improvement for reviewers 
and reduce some of the friction and remove some of the
non glamourous-task. Maybe it could be added to Mumi? Maybe a bot 
like Ofborg in NixOS could take care of some of the operation.

I haven't watched Arun presentation, I will search the list if the 
recording is up somewhere.

Guix is cool project, I am super happy to use it and would like to 
help more.

ph

[0]: https://issues.guix.gnu.org/55210

Ludovic Courtès <ludo@gnu.org> writes:

> Hello Guix!
>
> Following on the theme of patch review, I did some stats with 
> the
> attached tools on commits since 1.3.0:
>
>   • 20,489 commits were made since then;
>
>   • 4,476 were commits pushed on behalf of a non-committer;
>
>   • of these, half were pushed by 2 committers, out of 40ish.
>
> Some conclusions we can draw:
>
>   • We have a strong core development team, which I think is 
>   great
>     compared to many free software projects.
>
>     Perhaps the flip side of this is that we make too little 
>     space to
>     newcomers.  (I feel we’re almost the opposite of a typical
>     Git{Hub,Lab}-hosted project where drive-by contributions are 
>     common
>     and long-term commitment is rare.)
>
>   • Review work is severely lacking.  The manual reads (info 
>   "(guix)
>     Commit Access"):
>
>       […] the project keeps moving forward because committers 
>       not only push
>       their own awesome changes, but also offer some of their 
>       time
>       _reviewing_ and pushing other people’s changes.  As a 
>       committer,
>       you’re welcome to use your expertise and commit rights to 
>       help
>       other contributors, too!
>
>     Yet, most committers don’t allocate time to review and push 
>     other
>     people’s changes.
>
>     Why aren’t we, committers, not doing more review/apply work? 
>     Is it
>     too intimidating?  Would having a documented review 
>     checklist help?
>
>     If you’re not using Emacs, what actionable steps should we 
>     take with
>     mumi and other tools to help you (Arun made several 
>     proposals in
>     their Guix Days talk)?  If you are using Emacs, does 
>     debbugs.el have
>     shortcomings that make it a problem to review patches?
>
>   • We need to be able to renew committers.  There’s a process 
>   in place
>     to remove, at least temporarily, committers that have been 
>     inactive
>     for a year or more, and I think it’s good (info "(guix) 
>     Commit
>     Access").
>
>     Maybe we should also encourage committers who have “moved 
>     on” to let
>     the project know so we have a clearer picture of who’s 
>     in—meaning
>     available not just to commit their own occasional patches, 
>     but also
>     to help other contributors.
>
>     In addition to that, we need to encourage contributors who 
>     are not
>     committers yet, which obviously means reviewing and applying 
>     their
>     contributions in a timely fashion.  We need to grow prolific
>     contributors into leadership positions to they can become 
>     committers
>     and take part into this whole process.
>
> In short, we need to break out of a potentially vicious circle 
> where
> active members don’t make the work that would allow newcomers to 
> get
> more involved, at the risk of burning out themselves.
>
> Let’s make sure this project keeps striving for decades to come! 
> :-)
>
> Thoughts?
>
> Ludo’.
>
> (use-modules (git)
>              (git repository)
>              (git reference)
>              (git oid)
>              (git tag)
>              (git commit)
>              (git structs) 
>              ;signature-email, etc.
>              (guix git)
>              (srfi srfi-1)
>              (srfi srfi-26)
>              (ice-9 match)
>              (ice-9 vlist))
>
> (define commit-author*
>   (compose signature-name commit-author))
> (define commit-committer*
>   (compose signature-name commit-committer))
>
> (define-syntax-rule (false-if-git-error exp)
>   (catch 'git-error
>     (lambda () exp)
>     (const #f)))
>
> (define* (fold-commits proc seed repo
>                        #:key
>                        (start (reference-target
>                                (repository-head repo)))
>                        end)
>   "Call PROC on each commit of REPO, starting at START (an OID), 
>   and until
> END if specified."
>   (let loop ((commit (commit-lookup repo start))
>              (result seed))
>     (let ((parent (false-if-git-error (commit-parent commit))))
>       (if parent
>           (if (and end (oid=? (commit-id parent) end))
>               (proc parent result)
>               (loop parent (proc parent result)))
>           result))))
>
> (define (reviewers repo commits)
>   "Return a list of review count/committer pairs."
>   (define vhash
>     (fold (lambda (commit result)
>             (if (string=? (commit-author* commit)
>                           (commit-committer* commit))
>                 result
>                 (vhash-cons (commit-committer* commit) #t
>                             result)))
>           vlist-null
>           commits))
>
>   (define committers
>     (delete-duplicates
>      (fold-commits (lambda (commit result)
>                      (cons (commit-committer* commit)
>                            result))
>                    '()
>                    repo)))
>
>   (map (lambda (committer)
>          (cons (vhash-fold* (lambda (_ count)
>                               (+ 1 count))
>                             0
>                             committer
>                             vhash)
>                committer))
>        committers))
>
> (define (reviewer< r1 r2)
>   (match r1
>     ((count1 . name1)
>      (match r2
>        ((count2 . name2)
>         (< count1 count2))))))
>
> (define repo
>   (repository-open "."))
>
> (define commits
>   (commit-difference (commit-lookup repo
>                                     (reference-target 
>                                     (repository-head repo)))
>                      (commit-lookup
>                       repo
>                       (tag-target-id
>                        (tag-lookup
>                         repo
>                         (reference-target
>                          (reference-lookup repo 
>                          "refs/tags/v1.3.0")))))))


--
---
Thanks
ph

Re: On commit access, patch review, and remaining healthy

[Luis Felipe]

[-- Attachment #1.1: Type: text/plain, Size: 1086 bytes --]

Hi,

On Thursday, June 2nd, 2022 at 15:10, Ludovic Courtès <ludo@gnu.org> wrote:

> In addition to that, we need to encourage contributors who are not
> committers yet, which obviously means reviewing and applying their
> contributions in a timely fashion. We need to grow prolific
> contributors into leadership positions to they can become committers
> and take part into this whole process.

I used to have commit access a long time ago, but I eventually gave it up. I think the main reason was that I felt unconfortable having commit access to all Guix repositories (I only wanted access to guix-artwork at that time). I sometimes felt I was a security hole, specially when dealing with GPG, which I still find intimidating.

As for reviewing patches or just taking a look at reported issues against parts of Guix one is interested, it would be easier if there were some kind of subscription system. Say, I'd like to subscribe to issues on Python packages, Spanish translation, and guix-artwork. I know this kind of subscription thing has been mentioned before, though.

[-- Attachment #1.2: publickey - luis.felipe.la@protonmail.com - 0x12DE1598.asc --]
[-- Type: application/pgp-keys, Size: 1815 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 509 bytes --]

Feed on specific topic (public-inbox?)

[zimoun]

Hi,

On Thu, 02 Jun 2022 at 21:35, Luis Felipe <luis.felipe.la@protonmail.com> wrote:

> As for reviewing patches or just taking a look at reported issues
> against parts of Guix one is interested, it would be easier if there
> were some kind of subscription system. Say, I'd like to subscribe to
> issues on Python packages, Spanish translation, and guix-artwork. I
> know this kind of subscription thing has been mentioned before,
> though. 

As discussed in «Incentives for review» [1], using public-inbox, you can
filter using «dfn: match filename from diff» or even write more complex
queries [2].  For instance, read from your browser,

    https://yhetil.org/guix-patches/?q=dfn:python

And you can even follow a feed, for instance:

    https://yhetil.org/guix-patches/?q=dfn:python&x=A

Note that recent ’public-inbox’ comes from with the tool ’lei’. This
tool ’lei’ allows to query a public-inbox server and then fetch some
filtered messages to use it locally.  Currently, the Guix package is at
1.6 and it requires an update at 1.7; discussed here [3].  In this
message [4], you can find some Emacs workflow using public-inbox.


For guix-artwork, as a whole, I do not know what would be the best
query.

What is missing from Mumi is the use of Message-ID for an easy
referencing between the email tools, as discussed in [5].


As we discussed once, maybe it would help to have an “official“
public-inbox instance for the Guix project.  For instance, Org folks
uses a public-inbox instance [6] and switched 2 years ago [7].

From my point of view, public-inbox would fix some annoyance about the
“email workflow”.  Now the question is: who help in the maintenance? :-)


1: <https://yhetil.org/guix/87mtn56mzg.fsf_-_@inria.fr>
2: <https://public-inbox.org/meta/_/text/help/>
3: <https://yhetil.org/guix/877d6w96by.fsf@kolabnow.com>
4: <https://yhetil.org/guix/87bkwf956h.fsf@gmail.com>
5: <https://yhetil.org/guix/86sgbhz3fe.fsf@gmail.com>
6: <https://list.orgmode.org/orgmode/87ilyjuoed.fsf@bzg.fr/>
7: <https://list.orgmode.org/orgmode/87k10mt0n6.fsf@bzg.fr/>


Cheers,
simon

Re: Feed on specific topic (public-inbox?)

[zimoun]

On Fri, 03 Jun 2022 at 10:22, zimoun <zimon.toutoune@gmail.com> wrote:

> Note that recent ’public-inbox’ comes from with the tool ’lei’. This
> tool ’lei’ allows to query a public-inbox server and then fetch some
> filtered messages to use it locally.  Currently, the Guix package is at
> 1.6 and it requires an update at 1.7; discussed here [3].  In this
> message [4], you can find some Emacs workflow using public-inbox.

My bad, I am not enough up to date. :-)  Recent commit:
1e1de837190d1c08fae369a4cb3bf43a433abed1 from Wed Jun 1 provides
public-inbox v1.8, so it is now easy to test ’lei’. :-)


Cheers,
simon

Re: On commit access, patch review, and remaining healthy

[Ludovic Courtès]

Hi,

Brian Cully <bjc@spork.org> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>>     If you are using Emacs, does debbugs.el have
>>     shortcomings that make it a problem to review patches?

To be clear, the question was directed primarily at current committers.

> 1) It’d be nice if ‘M-x debbug-guix’ existed. I (briefly) looked at
> making such a command, thinking it’d be relatively straight-forward to
> just copy ‘debbugs-gnu’ and tweak some variables, but when it looked
> like it was going to take more than 10 minutes just to figure out how
> it was all put together I gave up as I can’t really dedicate time to
> it right now.

Try: C-u M-x debbugs-gnu.  From there you can choose ‘guix’,
‘guix-patches’, or anything else (info "(guix) Debbugs User
Interfaces").

> 2) IMHO, the documentation for debbugs is pretty poor. It mostly
> relies on GNUS documentation for things like mode help; and while this
> makes sense, GNUS is a big, complex package (which is why I use mu4e
> instead of it), and it really raises the barrier for understanding
> debbugs quite a lot.

I find the Debbugs User Guide, plus the docstrings and all, to be good
(info "(debbugs-ug) Top").

I use Gnus though so maybe there are issues I just don’t experience.

> 3) Even after reading all the documentation I could find, it doesn’t
> seem to work very well for an issue-tracker workflow. I still don’t
> know how to simply reply to a ticket. Let alone how to apply a patch
> to my tree (I did see documentation for how to do that to the Emacs
> source tree, but that’s not very useful — also, the key combinations
> feel very obtuse and hard for me to remember).
>
> So, right now, I just use it to browse, since I find it easier than
> mumi, but everything else happens with external tools. It’s very
> possible my n00b is showing right now and it’s easier to work with
> than it seems at first blush, but I’d say that means the documentation
> needs some dedicated, non-GNUS-oriented love. It would be very helpful
> if there were a HOWTO, like we used to do in the old days, with how to
> use it for simple tasks, and stick it in the info documentation and on
> the web.

Interesting.  Since I already used Gnus before, I didn’t have much to
learn when I started using debbugs.el.

I know some people here use debbugs.el with other email clients like
mu4e, so perhaps they can comment?  We could add guidance in the manual.

Thanks for your feedback!

Ludo’.

Re: On commit access, patch review, and remaining healthy

[Ludovic Courtès]

Hi,

Pier-Hugues Pellerin <ph@heykimo.com> skribis:

> As a new-new Guix user, I did find the review process or the time it
> takes really long.
> Maybe I've tackle too complex updates[0], I don't know but I don't
> have a clear path how to push it.

Yeah.  :-/

The long delays are mostly due to the lack of reviewers, but also, as
you point out, the more “complex” a patch series is, the more time it’ll
take before it’s reviewed (because it entails more work).

> Also, I don't want to say that one workflow is superior to the other
> but one thing that shines with theses forge
> is the automation and the simplicity to add tooling on pull request to
> reduce the burden to the reviewer:
>
> - linting
> - checks
> - commits log formatting check
> - CI jobs
> - codeowner assignment
> - build packages (partial or complete rebuild the world if needed)
> - license check
> - notification for stable pull request.

Yeah, I think there’s consensus on this.  :-)

> All theses are useful quality of life improvement for reviewers and
> reduce some of the friction and remove some of the
> non glamourous-task. Maybe it could be added to Mumi? Maybe a bot like
> Ofborg in NixOS could take care of some of the operation.

Yup.

> I haven't watched Arun presentation, I will search the list if the
> recording is up somewhere.

It’s right here:

  https://guix.gnu.org/en/blog/2022/online-guix-days-2022-announcement-2/

Thanks,
Ludo’.

Re: On commit access, patch review, and remaining healthy

[Ricardo Wurmus]

Hi Ludo,

> Interesting.  Since I already used Gnus before, I didn’t have much to
> learn when I started using debbugs.el.
>
> I know some people here use debbugs.el with other email clients like
> mu4e, so perhaps they can comment?  We could add guidance in the
> manual.

Since some time mu4e uses gnus for displaying and responding to emails,
so the old hack to make debbugs.el work with mu4e is no longer needed.

-- 
Ricardo

Re: On commit access, patch review, and remaining healthy

[Arun Isaac]

Hi all,

Tooling aside, at least for me, I think there is an important emotional
and psychological aspect to patch review. Maybe others share it too. So,
I'll speak up.

Sometimes, I don't review and commit patches because I feel like I am
not qualified to review them, and am afraid of pushing a "bad
commit". Guix has very high coding standards (which I very much
appreciate, BTW), but that means that there is a high cost of failure
and a pressure to live up to that high standard. This means that even if
I'm 99% sure of a commit, I tend to leave it to others because of that
nagging 1% doubt I have about some trivial aspect of the patch. The 1%
doubt could even be about really trivial things like indentation or the
name of a variable. In other words, perfectionism causes paralysis.

This excessive self-doubt is created by feeling like one doesn't
"belong" in the elite community of Guix hackers. This problem may be
alleviated somewhat by having more frequent (say, once in 3–4 months)
meetups and encouraging participation by shy people like myself. Having
human non-technical relationships with other members of the Guix
community can also go a long way. The WhereIsEveryone meetups already
help greatly. Perhaps Ricardo's idea of guix-mentors is another
direction worth pursuing.

If this same thread had come up a year or two ago, I would most likely
have remained silent. The only reason I feel alright talking today is
because recently I have got to know more members of the community
"face-to-face" (through online meetings), and feel more comfortable
opening up. I generally prefer text-only communication like email, but
sometimes, putting a human face on people and having a casual
conversation about nothing in particular, goes a long long way.

Cheers!
Arun

Re: On commit access, patch review, and remaining healthy

[Ludovic Courtès]

Hi Arun,

Arun Isaac <arunisaac@systemreboot.net> skribis:

> Tooling aside, at least for me, I think there is an important emotional
> and psychological aspect to patch review. Maybe others share it too. So,
> I'll speak up.

Thanks a lot for speaking up, your feedback is invaluable.

I did consider that the whole process could be intimidated; that even an
experienced contributor like you finds it intimidating is a red flag to
me.

> Sometimes, I don't review and commit patches because I feel like I am
> not qualified to review them, and am afraid of pushing a "bad
> commit". Guix has very high coding standards (which I very much
> appreciate, BTW), but that means that there is a high cost of failure
> and a pressure to live up to that high standard. This means that even if
> I'm 99% sure of a commit, I tend to leave it to others because of that
> nagging 1% doubt I have about some trivial aspect of the patch. The 1%
> doubt could even be about really trivial things like indentation or the
> name of a variable. In other words, perfectionism causes paralysis.

OK, understood.

I can think of two ways to reassure committers:

  1. By having clear reviewer check lists (you’d do that if you tick all
     the boxes, you’re fine);

  2. By improving automation—nothing new here: if there was a tick that
     says “applies without merge conflicts” and another one that read
     “builds fine”, anyone could lightheartedly hit the “merge” button.

#2 is going to take time I’m afraid, but at least #1 is actionable
(‘guix lint’ should help, too).

WDYT?  Are there other possibilities that come to mind?

> This excessive self-doubt is created by feeling like one doesn't
> "belong" in the elite community of Guix hackers. This problem may be
> alleviated somewhat by having more frequent (say, once in 3–4 months)
> meetups and encouraging participation by shy people like myself. Having
> human non-technical relationships with other members of the Guix
> community can also go a long way. The WhereIsEveryone meetups already
> help greatly. Perhaps Ricardo's idea of guix-mentors is another
> direction worth pursuing.

That’s a more subjective aspect, but a crucial one.  That perception of
an elite hacker community and the corresponding impostor syndrome are
problematic.  We long-time contributors should meditate that.

And yes, we should take advantage of the WhereIsEveryone meetups and
guix-mentors to get to know each other, to help each other, and to
demystify the whole thing.

> If this same thread had come up a year or two ago, I would most likely
> have remained silent. The only reason I feel alright talking today is
> because recently I have got to know more members of the community
> "face-to-face" (through online meetings), and feel more comfortable
> opening up. I generally prefer text-only communication like email, but
> sometimes, putting a human face on people and having a casual
> conversation about nothing in particular, goes a long long way.

Agreed.

Thank you for sharing how you feel about the process, it’s much
appreciated.

Ludo’.

Re: On commit access, patch review, and remaining healthy

[zimoun]

Hi,

On Mon, 06 Jun 2022 at 23:43, Ludovic Courtès <ludo@gnu.org> wrote:

> I can think of two ways to reassure committers:
>
>   1. By having clear reviewer check lists (you’d do that if you tick all
>      the boxes, you’re fine);

As pointed earlier by Arun in «Public guix offload server» [1], this
check list would imply some rebuilds and it can be difficult depending
on the resource at hand by the committer who reviews.

1: https://yhetil.org/guix/878rynh0yq.fsf@systemreboot.net


>   2. By improving automation—nothing new here: if there was a tick that
>      says “applies without merge conflicts” and another one that read
>      “builds fine”, anyone could lightheartedly hit the “merge” button.

From my understanding, one of the issue is where the committers push vs
where the users pull.  Basically, the push happen on master and the
changes are directly available to the users.  On one hand, it is nice
because the things are delivered faster.  On the other hand, a tiny
change can break many workflows – it is or could be a reason for
refraining pushing.

More eyes who review help, for sure.  But it is not straightforward as
we are often discussing. :-) More tools can also help, but AFAIK,
nothing is fully ready right now.

What remain is: not push to the production branch. :-) Maybe, we could
push to a branch “unstable” **automatically** merged every week to the
branch “stable” and by default user pull “stable”.  One week let the
time to build by the CI, check everything is fine and fix otherwise.  It
reduces a bit the pressure on the committers, IMHO.

We – from core developers to user just wanting the things done – are all
pulling the same branch.  This branch cannot satisfy in the same time
all the constraints; is the current push/pull branch model satisfying
the best optimum with the resource and tools at hand?


Cheers,
simon

Re: On commit access, patch review, and remaining healthy

[Efraim Flashner]

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

On Fri, Jun 03, 2022 at 09:37:36PM +0200, Ludovic Courtès wrote:
> Hi,
> 
> Brian Cully <bjc@spork.org> skribis:
> 
> > Ludovic Courtès <ludo@gnu.org> writes:
> >
> >>     If you are using Emacs, does debbugs.el have
> >>     shortcomings that make it a problem to review patches?
> 
> To be clear, the question was directed primarily at current committers.
> 
> > 1) It’d be nice if ‘M-x debbug-guix’ existed. I (briefly) looked at
> > making such a command, thinking it’d be relatively straight-forward to
> > just copy ‘debbugs-gnu’ and tweak some variables, but when it looked
> > like it was going to take more than 10 minutes just to figure out how
> > it was all put together I gave up as I can’t really dedicate time to
> > it right now.
> 
> Try: C-u M-x debbugs-gnu.  From there you can choose ‘guix’,
> ‘guix-patches’, or anything else (info "(guix) Debbugs User
> Interfaces").
> 
> > 2) IMHO, the documentation for debbugs is pretty poor. It mostly
> > relies on GNUS documentation for things like mode help; and while this
> > makes sense, GNUS is a big, complex package (which is why I use mu4e
> > instead of it), and it really raises the barrier for understanding
> > debbugs quite a lot.
> 
> I find the Debbugs User Guide, plus the docstrings and all, to be good
> (info "(debbugs-ug) Top").
> 
> I use Gnus though so maybe there are issues I just don’t experience.
> 
> > 3) Even after reading all the documentation I could find, it doesn’t
> > seem to work very well for an issue-tracker workflow. I still don’t
> > know how to simply reply to a ticket. Let alone how to apply a patch
> > to my tree (I did see documentation for how to do that to the Emacs
> > source tree, but that’s not very useful — also, the key combinations
> > feel very obtuse and hard for me to remember).
> >
> > So, right now, I just use it to browse, since I find it easier than
> > mumi, but everything else happens with external tools. It’s very
> > possible my n00b is showing right now and it’s easier to work with
> > than it seems at first blush, but I’d say that means the documentation
> > needs some dedicated, non-GNUS-oriented love. It would be very helpful
> > if there were a HOWTO, like we used to do in the old days, with how to
> > use it for simple tasks, and stick it in the info documentation and on
> > the web.
> 
> Interesting.  Since I already used Gnus before, I didn’t have much to
> learn when I started using debbugs.el.
> 
> I know some people here use debbugs.el with other email clients like
> mu4e, so perhaps they can comment?  We could add guidance in the manual.

As someone who has never used debbugs or emacs I find it daunting to try
to add it into my workflow. Currently I am subscribed to guix-patches
and I dump it into my guix-devel mailing list. I read my mail using mutt
and will just pipe the patches to git to apply them and try them out
that way. After years and years of this I'm pretty happy with this
aspect of my workflow, but finding older patches can be more
challenging. And in our case older can be only a week old.

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

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

Re: On commit access, patch review, and remaining healthy

[Ludovic Courtès]

Hi Efraim,

Efraim Flashner <efraim@flashner.co.il> skribis:

> As someone who has never used debbugs or emacs I find it daunting to try
> to add it into my workflow. Currently I am subscribed to guix-patches
> and I dump it into my guix-devel mailing list. I read my mail using mutt
> and will just pipe the patches to git to apply them and try them out
> that way. After years and years of this I'm pretty happy with this
> aspect of my workflow, but finding older patches can be more
> challenging. And in our case older can be only a week old.

The manual mentions the two web interfaces in addition to Emacs:

  https://guix.gnu.org/manual/devel/en/html_node/Debbugs-User-Interfaces.html

Do you or would you use them to keep track of pending patches?

Perhaps a CLI taking advantage of mumi’s HTTP interface (or using
Guile-Debbugs) would work better?  Like:

  mumi new          # list new issues
  mumi fetch 1234   # fetch patches of issue #1234
  mumi close 1234
  …

WDYT?

Ludo’.

Re: On commit access, patch review, and remaining healthy

[Giovanni Biscuolo]

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

Hi Simon and all,

just a quick note about myself: I'm (still) not contributing with patch
reviews (and in general contributing too little) because in this period
of my "work life" I have little time, but things will hopefully
change...

IMHO the curent tooling is helpful and usable with a little bit of
studying (git, patch management, guix lint et al...) and effort,
obviously more tools could help more users (debbugs for vim (?!?), mumi
CLI) but the real hard work is not at the interface level, it's at
"content" level AND at "machine power" available resources

IMHO the whole process should not be automated more than this (I mean,
more than making as easy as possible patch application using the
reviewer preferred tools): review is a human-only job after all... let's
just say reviwers are giving their fair contribution to the verification
of code above stage0... at a higher level (stage99? :-) )

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

>> I can think of two ways to reassure committers:
>>
>>   1. By having clear reviewer check lists (you’d do that if you tick all
>>      the boxes, you’re fine);
>
> As pointed earlier by Arun in «Public guix offload server» [1], this
> check list would imply some rebuilds and it can be difficult depending
> on the resource at hand by the committer who reviews.

I can confirm that without a local offload build server Guix package
development and review is unpractical: one of the first things I had to
do in order to not negatively impact my working evironment was to buy a
(used) machine and set it up as a local offload build server, that
improved my (rare, as I said) work on guix package devel a lot.

Maybe we should clarify this in the contributing section of the manual?
Maybe we should warn users that to contribute you /first/ have to get
enough machine CPU+memory power, possibly on an offload server (used
computers can do great things)?

IMHO local distributed build for patch review purposes is better than
a centralized one, also as a sort of reproducibility test, no?

> 1: https://yhetil.org/guix/878rynh0yq.fsf@systemreboot.net

AFAIU quickly reading that thread (dated 2021-10-20) a public offload
build server "as is" is too much a security risk: interested people
should re-read that thread, Tobias Geerinckx-Rice explained very well
the threat model.  I would never use a shared offload build server or
one not on bare-metal AND under my direct physical control

Ludo' told us it should be possible to use a remote daemon without
mutual trust between machines (id:87o8782g6q.fsf@gnu.org) and that we
could have an HTTP bridge or API: what's the situation today?

Anyway, I'd prefer a third build farm to cross-challenge substitutes
than a public shared offload build server. :-O

[...]

> What remain is: not push to the production branch. :-) Maybe, we could
> push to a branch “unstable”

AFAIU this have little to do with patch review, unless you imply that
committers can push to "unstable" with "less patch review" :-D

> **automatically** merged every week to the branch “stable” and by
> default user pull “stable”.  One week let the time to build by the CI,
> check everything is fine and fix otherwise.

This means that if the fix is not committed (rebased?) in that weekly
timerfame the problematic patch is automatically pushed to stable
without a fix; also we'll have that problematic commit in stable anyway
(affecting users like me that are "pinning" specific channels?), unless
we rebase "unstable"... "manually": am I wrong?

With a patch-dedicated git branch the reviewer can work at his preferred
pace on that patch, rebasing /when/ (not if) needed, without the risk a
problematic commit will be auto-committed.

...and yes: IMHO a linear git history avoiding merges is very useful.

> It reduces a bit the pressure on the committers, IMHO.

It raises a bit the pressure on the maintainers, IMHO :-)

IMVHO there is no effective "workarond" from proper patch review work.

I understand there is a certain "entrance barrier" to become patch
reviewer, but I'm afraid we cannot lower it more than the current
situation except for the offload build server and more tolling options.

[...]

Than you founders!
Thank you maintainers!
Thank you committers!

...and than you Simon and all for this constructive discussions!

Happy Hacking! Gio'


P.S. when considering how much easy or difficult is to contribute to
Guix as a software distribution we should also look at the contribution
workflow model and tooling of other distributions, rolling and
not-rolling (e.g. Opensuse, Debian): AFAIU we are not so bad at this
compared to others (except probably the number of "package maintainers")
:-D

-- 
Giovanni Biscuolo

Xelera IT Infrastructures

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

Re: On commit access, patch review, and remaining healthy

[Giovanni Biscuolo]

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

Hi Arun,

Arun Isaac <arunisaac@systemreboot.net> writes:

> Tooling aside, at least for me, I think there is an important emotional
> and psychological aspect to patch review. Maybe others share it too. So,
> I'll speak up.

Thanks a lot for your speak up!

> Guix has very high coding standards

Do you think other software distribution do have less high coding
standards?

[...]

> but that means that there is a high cost of failure

please can you expand on this?  What is that cost of failure?

> and a pressure to live up to that high standard. This means that even
> if I'm 99% sure of a commit, I tend to leave it to others because of
> that nagging 1% doubt I have about some trivial aspect of the
> patch. The 1% doubt could even be about really trivial things like
> indentation or the name of a variable. In other words, perfectionism
> causes paralysis.

generally speaking I really understand your feeling (the "impostor
syndrome" as Ludo' say) but please do not be paralyzed by this feeling,
do /not/ have fear to make a mistake, we (pluralis majestatis :-D ) are
not here to judge you

specifically speaking, IMHO in cases like this you should send an
email-reply to that bug (patch) explaining the 1% you are unsure of

> This excessive self-doubt is created by feeling like one doesn't
> "belong" in the elite community of Guix hackers

I undestand there are many meanings of "elite" [1], but if it's an elite
it is /for sure/ a very open group of persons

[...]

Thanks!  Gio'


[1] https://www.merriam-webster.com/dictionary/elite

-- 
Giovanni Biscuolo

Xelera IT Infrastructures

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

Re: On commit access, patch review, and remaining healthy

[Giovanni Biscuolo]

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

Hi Ludo'

Ludovic Courtès <ludo@gnu.org> writes:

[...]

> OK, understood.
>
> I can think of two ways to reassure committers:
>
>   1. By having clear reviewer check lists (you’d do that if you tick all
>      the boxes, you’re fine);

also a description of the review process used by you and other
experienced patch reviewers could be much useful: it's more /how/ do you
check al the aspects of a patch (it's not only running "guix lint"
obviously) than /what/ do you check, IMHO

maybe this could be a specific chapter of the cookbook, maybe some
experienced patch reviewer could make a sort of (reharshed) "live patch
review" video of some patch representing interesting "class of patches"
reviewers may encounter (upstream updates, changes in Guix package
structure, patches to fix building issues...)

if available I'd also attend a paid "Contributing Guix" course, since it
would be /very/ useful to my work... but that's out of scope here, I
think

>   2. By improving automation—nothing new here: if there was a tick that
>      says “applies without merge conflicts” and another one that read
>      “builds fine”, anyone could lightheartedly hit the “merge”
>      button.

OK but this can also be achieved now without automation, just by
commenting the patch submission so that committers can "reuse" the work
already done by others; a check-list can help for sure: could it be
auto-generated by debbugs via a mail-template on receiving a message in
the patch-list?

> #2 is going to take time I’m afraid, but at least #1 is actionable
> (‘guix lint’ should help, too).
>
> WDYT?  Are there other possibilities that come to mind?

is it possible to automate "guix lint" (and add it as git hook before
commit?) only for the package(s) specific to a patch-set? [1]

[...]

> And yes, we should take advantage of the WhereIsEveryone meetups and
> guix-mentors to get to know each other, to help each other, and to
> demystify the whole thing.

I'd not miss the chance to someway document important things said during
meetings/IRC/email-messages useful to demystify the whole thing: I know
it's more work for some of you, but hopefully for less work in the
future

[...]

Thanks!  Gio'


[1] that is: is there a command that given a patch is able to give us a
list of affected guix packages?

-- 
Giovanni Biscuolo

Xelera IT Infrastructures

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

Re: On commit access, patch review, and remaining healthy

[Efraim Flashner]

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

On Tue, Jun 07, 2022 at 05:11:48PM +0200, Ludovic Courtès wrote:
> Hi Efraim,
> 
> Efraim Flashner <efraim@flashner.co.il> skribis:
> 
> > As someone who has never used debbugs or emacs I find it daunting to try
> > to add it into my workflow. Currently I am subscribed to guix-patches
> > and I dump it into my guix-devel mailing list. I read my mail using mutt
> > and will just pipe the patches to git to apply them and try them out
> > that way. After years and years of this I'm pretty happy with this
> > aspect of my workflow, but finding older patches can be more
> > challenging. And in our case older can be only a week old.
> 
> The manual mentions the two web interfaces in addition to Emacs:
> 
>   https://guix.gnu.org/manual/devel/en/html_node/Debbugs-User-Interfaces.html
> 
> Do you or would you use them to keep track of pending patches?
> 
> Perhaps a CLI taking advantage of mumi’s HTTP interface (or using
> Guile-Debbugs) would work better?  Like:
> 
>   mumi new          # list new issues
>   mumi fetch 1234   # fetch patches of issue #1234
>   mumi close 1234
>   …
> 
> WDYT?

Of the little bit I've seen of debbugs I think I really want is to have
a mailbox folder with just the patches and bugs in it (finally set this
up yesterday) and to have the threads automatically marked as read after
the bug has been closed. I don't know anything about usertags to make
use of them, so it's not something I've ever missed.

Since I've now split the mailboxes I have less than 300 unread messages
from guix-devel and almost 7000 unread in my shared guix-patches/bugs-guix
mailbox. I would gladly trade that mailbox for something in the terminal
that connected to debbugs directly for the autoupdating of closed bugs
and would still allow me to pipe the patches themselves to apply them to
the git repo.

It might be what I'm looking for is a minimal emacs config that is just
a frontend to debbugs and lets me use all my normal tools (vim, msmtp,
etc.) for everything else.

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

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

Re: On commit access, patch review, and remaining healthy

[Ludovic Courtès]

Efraim Flashner <efraim@flashner.co.il> skribis:

> On Tue, Jun 07, 2022 at 05:11:48PM +0200, Ludovic Courtès wrote:

[...]

>> The manual mentions the two web interfaces in addition to Emacs:
>> 
>>   https://guix.gnu.org/manual/devel/en/html_node/Debbugs-User-Interfaces.html
>> 
>> Do you or would you use them to keep track of pending patches?

You didn’t reply to that one but I suppose it’s “no”.

> Of the little bit I've seen of debbugs I think I really want is to have
> a mailbox folder with just the patches and bugs in it (finally set this
> up yesterday) and to have the threads automatically marked as read after
> the bug has been closed. I don't know anything about usertags to make
> use of them, so it's not something I've ever missed.
>
> Since I've now split the mailboxes I have less than 300 unread messages
> from guix-devel and almost 7000 unread in my shared guix-patches/bugs-guix
> mailbox. I would gladly trade that mailbox for something in the terminal
> that connected to debbugs directly for the autoupdating of closed bugs
> and would still allow me to pipe the patches themselves to apply them to
> the git repo.
>
> It might be what I'm looking for is a minimal emacs config that is just
> a frontend to debbugs and lets me use all my normal tools (vim, msmtp,
> etc.) for everything else.

Well, you could do use Emacs + debbugs.el to browse the list of bugs,
but coming up with a simple CLI is doable.

Ludo’.

Re: On commit access, patch review, and remaining healthy

[Arun Isaac]

Hi Giovanni,

>> Guix has very high coding standards
>
> Do you think other software distribution do have less high coding
> standards?

I meant that Guix has very high coding/packaging standards in the
following senses:

- We prefer to not bundle dependencies. Tracking out bundled
  dependencies, git submodules, etc. and figuring out how to rewire
  everything so it works with unbundled dependencies can be really
  tricky for some packages.
  
- We build strictly from source.

- We aim to package tests for all packages.

- We have strict conventions for commit messages. Our commit message
  Changelog is a strange dated practice from the time before good
  version control systems. I can live with it, but not everyone likes
  it. Let's just say I've heard complaints about it offlist.

- We don't just list the main license of a package. We trace out the
  license of each and every file, if they are different from the main
  license.

- Our synopses and descriptions are not casually copy-pasted from the
  project website. We try to rewrite and improve on them if necessary.

- We have to be careful about pushing changes that will cause too many
  rebuilds. We have a core-updates process for that.

I could go on forever, but you get the idea. I do agree with most of
these coding standards, and have no problem with following them. But, it
does add some pressure to the process.

I don't have much experience with other distributions except for Arch
Linux and that too only with the AUR (Arch User Repository). The AUR was
more of a wiki than a centrally curated repository. Anyone can
contribute packages, and there is no review. Most packages don't run
tests, and the standards are generally low.

>> but that means that there is a high cost of failure
>
> please can you expand on this?  What is that cost of failure?

The cost of failure is mostly in the mind. A commit is something that
has your name on it and lives on in the repo history *forever*. So, it
better be good. That's the pressure. I do have one or two commits in the
guix repo with badly borked commit messages.

>> and a pressure to live up to that high standard. This means that even
>> if I'm 99% sure of a commit, I tend to leave it to others because of
>> that nagging 1% doubt I have about some trivial aspect of the
>> patch. The 1% doubt could even be about really trivial things like
>> indentation or the name of a variable. In other words, perfectionism
>> causes paralysis.
>
> generally speaking I really understand your feeling (the "impostor
> syndrome" as Ludo' say) but please do not be paralyzed by this feeling,
> do /not/ have fear to make a mistake, we (pluralis majestatis :-D ) are
> not here to judge you

I know. ;-)

> specifically speaking, IMHO in cases like this you should send an
> email-reply to that bug (patch) explaining the 1% you are unsure of

Yeah, I would. But, often that 1% is too nitpicky to be worth
reporting. Sometimes, I fix the 1% myself and push. But often, I confess
that I just leave it to other committers.

>> This excessive self-doubt is created by feeling like one doesn't
>> "belong" in the elite community of Guix hackers
>
> I undestand there are many meanings of "elite" [1], but if it's an
> elite

I meant "elite" in the sense that Guix hackers are among the most
skilled programmers I've known.

> it is /for sure/ a very open group of persons

Definitely, I love the community! That's why I'm still here after all
these years!

Cheers! :-)
Arun

Re: On commit access, patch review, and remaining healthy

[Arun Isaac]

Hi Ludo,

> I can think of two ways to reassure committers:
>
>   1. By having clear reviewer check lists (you’d do that if you tick all
>      the boxes, you’re fine);
>
>   2. By improving automation—nothing new here: if there was a tick that
>      says “applies without merge conflicts” and another one that read
>      “builds fine”, anyone could lightheartedly hit the “merge” button.
>
> #2 is going to take time I’m afraid, but at least #1 is actionable
> (‘guix lint’ should help, too).
>
> WDYT?  Are there other possibilities that come to mind?

Nothing very specific comes to mind immediately. When I do have specific
ideas, I'll be sure to write to guix-devel about them.

I'm largely happy with the direction the Guix community is headed with
teams, mentors and better tooling. So, at this point, it will be good to
progressively experiment with these ideas and see where it leads.

Regards,
Arun

Re: On commit access, patch review, and remaining healthy

[Giovanni Biscuolo]

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

Hi Arun,

thank you for your detailed analysis!

Arun Isaac <arunisaac@systemreboot.net> writes:

[...]

> I meant that Guix has very high coding/packaging standards in the
> following senses:
>
> - We prefer to not bundle dependencies. Tracking out bundled
>   dependencies, git submodules, etc. and figuring out how to rewire
>   everything so it works with unbundled dependencies can be really
>   tricky for some packages.

more than tricky: sometimes /impossible/

this is a /huge/ complex issue (I know you know) that some upstream
developers simply ignore; in a sense I feel that Guix was developed
/also/ to help solve this issue and avoid programmers to invent too
creative ways to distribute theis software dependencies

...but it's a hard work, it needs... zen discipline

anyway: AFAIU all of us do agree that this requirement is a Good Thing™
and we have to keep it as a "core requirement"

also, AFAIU there is no way Guix maintainers can help in making this
process easier or automatic: right?

> - We build strictly from source.

This is also a requirement now adopted by many other distributions, at
least all the ones in https://reproducible-builds.org/who/projects/

> - We aim to package tests for all packages.

is there something different that should be done?

do this requirement (is it a requirement?) need to be better documented
or discussed?

> - We have strict conventions for commit messages. Our commit message
>   Changelog is a strange dated practice from the time before good
>   version control systems. I can live with it, but not everyone likes
>   it. Let's just say I've heard complaints about it offlist.

AFAIU this is a requirement Guix inherits from GNU (being it a GNU
project)

I don't remember the ratio for this requirement but AFAIU it made sense
to me when I read that.

I just hope this requirement is refraining people to contribute and to
review patches.

Maybe we could help users not using Emacs with other editor-related
snippets in [~/src/guix/]etc/snippets? (I don't know other editors
templating systems)

> - We don't just list the main license of a package. We trace out the
>   license of each and every file, if they are different from the main
>   license.

Is it possible to write an helper tool?!?  I have still not searched for
one such tool, maybe other distributions do have such a tool?

BTW AFAIU this is a legal requirement we cannot avoid, even if sometimes
it's avoided by some upstream author

> - Our synopses and descriptions are not casually copy-pasted from the
>   project website. We try to rewrite and improve on them if necessary.

AFAIK similar requirements are "enforced" by all other distributions

> - We have to be careful about pushing changes that will cause too many
>   rebuilds. We have a core-updates process for that.

Could this be automated by a pre-commit hook?

[...]

>> please can you expand on this?  What is that cost of failure?
>
> The cost of failure is mostly in the mind. A commit is something that
> has your name on it and lives on in the repo history *forever*. So, it
> better be good. That's the pressure. I do have one or two commits in the
> guix repo with badly borked commit messages.

OK thanks, I understand

[...]

>> specifically speaking, IMHO in cases like this you should send an
>> email-reply to that bug (patch) explaining the 1% you are unsure of
>
> Yeah, I would. But, often that 1% is too nitpicky to be worth
> reporting. Sometimes, I fix the 1% myself and push. But often, I confess
> that I just leave it to other committers.

I don't imagine a specific use case, but nevermind

[...]

Thank you! Gio'

-- 
Giovanni Biscuolo

Xelera IT Infrastructures

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

Re: On commit access, patch review, and remaining healthy

[Efraim Flashner]

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

On Fri, Jun 10, 2022 at 02:27:44PM +0200, Giovanni Biscuolo wrote:
> Hi Arun,
> 
> thank you for your detailed analysis!
> 
<..snip..>
> I just hope this requirement is refraining people to contribute and to
> review patches.

I'll reword this as "I just hope this requirement isn't preventing
people from contributing and reviewing packages"

> Maybe we could help users not using Emacs with other editor-related
> snippets in [~/src/guix/]etc/snippets? (I don't know other editors
> templating systems)

I'm not using emacs but I've found etc/committer.scm to be quite
helpful, and I'll often rework parts of my workflow so that I can run
:!etc/committer.scm from within vim.

I thought we had a yasnippet plugin for vim packaged, but it seems we
don't yet.

> 
<..snip..>
> 
> Thank you! Gio'
> 



-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

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

Re: On commit access, patch review, and remaining healthy

[Maxime Devos]

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

Giovanni Biscuolo schreef op vr 10-06-2022 om 14:27 [+0200]:
> > - Our synopses and descriptions are not casually copy-pasted from
> the
> >    project website. We try to rewrite and improve on them if
> necessary.
> 
> AFAIK similar requirements are "enforced" by all other distributions

They aren't by the special-purpose crates.io and pypi.org distributions
AFACIT, though maybe including those distributions here is a bit too
nitpicky?  This rule has also been neglected for the Rust crates in
Guix.

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

Re: On commit access, patch review, and remaining healthy

[Giovanni Biscuolo]

Efraim Flashner <efraim@flashner.co.il> writes:

> On Fri, Jun 10, 2022 at 02:27:44PM +0200, Giovanni Biscuolo wrote:

[...]

>> I just hope this requirement is refraining people to contribute and to
>> review patches.
>
> I'll reword this as "I just hope this requirement isn't preventing
> people from contributing and reviewing packages"

ROTFL, my bad inversion logic!

>> Maybe we could help users not using Emacs with other editor-related
>> snippets in [~/src/guix/]etc/snippets? (I don't know other editors
>> templating systems)
>
> I'm not using emacs but I've found etc/committer.scm to be quite
> helpful, and I'll often rework parts of my workflow so that I can run
> :!etc/committer.scm from within vim.
>
> I thought we had a yasnippet plugin for vim packaged, but it seems we
> don't yet.
>
>> 
> <..snip..>
>> 
>> Thank you! Gio'
>> 
>
>
>
> -- 
> Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
> GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
> Confidentiality cannot be guaranteed on emails sent or received unencrypted
-- 
Giovanni Biscuolo

Xelera IT Infrastructures

Re: On commit access, patch review, and remaining healthy

[Giovanni Biscuolo]

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

Efraim Flashner <efraim@flashner.co.il> writes:

[...]

> I'm not using emacs but I've found etc/committer.scm to be quite
> helpful, and I'll often rework parts of my workflow so that I can run
> :!etc/committer.scm from within vim.

uh thanks! I didn't know about committer.scm

please is there some documentation about the content of etc/?

I searched "committer.scm" with grep in the whole repo but did not find
info

> I thought we had a yasnippet plugin for vim packaged, but it seems we
> don't yet.

are there yasnippet plugins for vim?

[...]

Thanks! Gio'

-- 
Giovanni Biscuolo

Xelera IT Infrastructures

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

Re: On commit access, patch review, and remaining healthy

[Thiago Jung Bauermann]

Hello,

Giovanni Biscuolo <g@xelera.eu> writes:

> Arun Isaac <arunisaac@systemreboot.net> writes:
>
>> - We have strict conventions for commit messages. Our commit message
>>   Changelog is a strange dated practice from the time before good
>>   version control systems. I can live with it, but not everyone likes
>>   it. Let's just say I've heard complaints about it offlist.
>
> AFAIU this is a requirement Guix inherits from GNU (being it a GNU
> project)

Yes, but not all projects use it anymore. For instance, GCC still does
but glibc and GDB have dropped them. I couldn't figure out binutils.
Some commits have them, some don't. It seems that nowadays it's more
what you'd call “guidelines” than actual rules…

FWIW I also think they're obsolete and would prefer not to have to do
them. Though most Guix patches I work on are small so it's not actually
too much trouble after you get used to it.

But I do think it's one more source of “friction” for new contributors,
and one more thing for us to require that they get right.

> I don't remember the ratio for this requirement but AFAIU it made sense
> to me when I read that.

There's one in the GNU Coding Standards¹:

    Keep a change log to describe all the changes made to program source
    files. The purpose of this is so that people investigating bugs in
    the future will know about the changes that might have introduced
    the bug. Often a new bug can be found by looking at what was
    recently changed. More importantly, change logs can help you
    eliminate conceptual inconsistencies between different parts of a
    program, by giving you a history of how the conflicting concepts
    arose, who they came from, and why the conflicting changes were
    made.

Personally, I think nowadays this purpose is better fulfilled by
good commit messages and git blame. Especially with an editor that makes
it easy to use them to navigate through history (such as Emacs, but
certainly others as well).

> Maybe we could help users not using Emacs with other editor-related
> snippets in [~/src/guix/]etc/snippets? (I don't know other editors
> templating systems)

The binutils-gdb repo has a Python script to generate a skeleton
ChangeLog. I don't know how well it would work for Scheme patches:

https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=contrib/mklog.py;hb=HEAD

-- 
Thanks
Thiago

¹ https://www.gnu.org/prep/standards/standards.html#Change-Logs

Re: On commit access, patch review, and remaining healthy

[Ludovic Courtès]

Hi,

Thiago Jung Bauermann <bauermann@kolabnow.com> skribis:

> The binutils-gdb repo has a Python script to generate a skeleton
> ChangeLog. I don't know how well it would work for Scheme patches:
>
> https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=contrib/mklog.py;hb=HEAD

I believe the ‘etc/committer.scm’ scripts plays a similar role (it’s
even more sophisticated!), but perhaps it’s too little known.

Ludo’.

Re: On commit access, patch review, and remaining healthy

[Ricardo Wurmus]

> Date: Fri, 10 Jun 2022 14:27:44 +0200
> From: Giovanni Biscuolo <g@xelera.eu>
> To: Arun Isaac <arunisaac@systemreboot.net>, Guix Devel
> 	<guix-devel@gnu.org>
> Cc: GNU Guix maintainers <guix-maintainers@gnu.org>
> Subject: Re: On commit access, patch review, and remaining healthy
> Message-ID: <87o7z0itz3.fsf@xelera.eu>
> Content-Type: text/plain; charset="utf-8"
>
>> - We build strictly from source.
>
> This is also a requirement now adopted by many other distributions, at
> least all the ones in https://reproducible-builds.org/who/projects/

NixOS is on the list, but they don’t have this requirement.  That’s why
they have Java packages that are little more than the upstream jars, or
have packages with bundled dependencies (e.g. vendored jars).

-- 
Ricardo

Re: On commit access, patch review, and remaining healthy

[Ricardo Wurmus]

>> - We have strict conventions for commit messages. Our commit message
>>   Changelog is a strange dated practice from the time before good
>>   version control systems. I can live with it, but not everyone likes
>>   it. Let's just say I've heard complaints about it offlist.
>
> AFAIU this is a requirement Guix inherits from GNU (being it a GNU
> project)
>
> I don't remember the ratio for this requirement but AFAIU it made sense
> to me when I read that.
>
> I just hope this requirement is refraining people to contribute and to
> review patches.
>
> Maybe we could help users not using Emacs with other editor-related
> snippets in [~/src/guix/]etc/snippets? (I don't know other editors
> templating systems)

We have an editor-agnostic tool in ./etc/committer.scm.

-- 
Ricardo

Re: On commit access, patch review, and remaining healthy

[Giovanni Biscuolo]

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

Hi Ricardo and all,

following this discussion, it came to my mind a great presentation made
by Prot:

https://protesilaos.com/codelog/2021-12-21-emacsconf2021-freedom/
«How Emacs made me appreciate software freedom»

especially the "You can't be an Emacs tourist" part; I think that
similar arguments can be adapted to a "(Guix?) Software developer can't
be a repro+bootstrapping tourist" (to fully unserstand my analogy please
read or listen to Prot presentation)

concerning this discussion, this is probably the most interesting part:

--8<---------------cut here---------------start------------->8---

Now you may wonder why do I mention those things?  Shouldn't we make
Emacs easier for everyone?  Yes, we should make everything as simple as
possible.  Though that still does not refashion Emacs into something
entirely different.  We continue to have a potent tool at our disposal
that we must treat with the requisite respect.  Take, for instance, the
various frameworks that set up Emacs in an opinionated way so that
newcomers get everything set up for them out-of-the-box.  There is
nothing wrong with those frameworks.  In fact, a large part of the
community uses them to great effect.  However, the point stands: even
after every package has been set up for you, you still have to put in
the work in making use of your newfound computing freedom.

--8<---------------cut here---------------end--------------->8---

Ricardo Wurmus <rekado@elephly.net> writes:

[...]

>>> - We build strictly from source.
>>
>> This is also a requirement now adopted by many other distributions, at
>> least all the ones in https://reproducible-builds.org/who/projects/
>
> NixOS is on the list, but they don’t have this requirement.  That’s why
> they have Java packages that are little more than the upstream jars,

good point Ricardo, the very moment I started replying I had it in my
mind but forgot to write it

I guess that all experienced packagers or maintainers well understands
what's needed in order to get a reproducible AND bootstrappable package:
almost all of the "constraints" Guix "impose" to packagers and
contributors depends from this... let's call them "golden rules of
software security"?

I just feel sometimes it's hard for newcomers to understand this,
especially considering that unfortunately both some projects in that
list (https://reproducible-builds.org/who/projects/) and some (some?)
upstream developers do not care much about them

the "tag line" of https://reproducible-builds.org/ is

--8<---------------cut here---------------start------------->8---

Reproducible builds are a set of software development practices that
create an independently-verifiable path from source to binary code.

--8<---------------cut here---------------end--------------->8---

honestly I did not study all the reproducible-builds.org documentation,
but it's impossible to me to understand how a packaged upstream jar can
be considered reproducible (and bootstrappable); maybe distros like
NixOS are still slowly transitioning to a full reproducible build
workflow?

IMHO the simple fact that (some, one?) projects listed on
reproducible-builds.org are still bundling binaries in their packages
it's too confusing for newcomers

> or have packages with bundled dependencies (e.g. vendored jars).

bundling binaries it's (is it?) for sure against the definition of a
reproducible build, but what about bundling (source) dependencies?

AFAIU not to bundle (source) dependencies is an additional Guix
requirement (and it is a Good Thing™): do I miss something?

Thanks! Gio'

-- 
Giovanni Biscuolo

Xelera IT Infrastructures

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

Re: On commit access, patch review, and remaining healthy

[Maxime Devos]

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

Giovanni Biscuolo schreef op zo 12-06-2022 om 11:42 [+0200]:
> > or have packages with bundled dependencies (e.g. vendored jars).
> 
> bundling binaries it's (is it?) for sure against the definition of a
> reproducible build, but what about bundling (source) dependencies?
> 
> AFAIU not to bundle (source) dependencies is an additional Guix
> requirement (and it is a Good Thing™): do I miss something?

FWIW, sometimes the bundled ‘source’ dependencies contain bundled
binaries of their own.  So while AFAICT not strictly necessary for
reproducible builds, unbundling ‘source dependencies’ makes ensuring
reproducibility(*) much more convenient.

(*) i.e., the non-trivial kind of reproducibility, where things are
actually built from source instead of copying binaries.

> honestly I did not study all the reproducible-builds.org
> documentation,
> but it's impossible to me to understand how a packaged upstream jar
> can be considered reproducible (and bootstrappable); maybe distros
> like NixOS are still slowly transitioning to a full reproducible
> build workflow?

It's ‘reproducible’ in the trivial sense that you can ‘reproduce’ a
scientific paper by putting it a photocopier.  That way, you can
reproduce the results, but you cannot confirm whether these results
were correct.

Greetings,
Maxime.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

Re: On commit access, patch review, and remaining healthy

[Giovanni Biscuolo]

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

Hi Maxime and all,

I'm so sorry this discussion is shifting to the actual meaning of
reproducible... especially since Maxime and many of us know it very well

I'm sure 95% of people I know would not understand me if I tell them
that their software should be reproducible, they should study a little
bit (effort) what software is to really understand what I mean... or
Just Trust Us™

Maxime Devos <maximedevos@telenet.be> writes:

[...]

> It's ‘reproducible’ in the trivial sense that you can ‘reproduce’ a
> scientific paper by putting it a photocopier.

Maxime I have a question for you please: do you really think that in the
NixOS community (or any other project mentioned in
https://reproducible-builds.org/who/projects/) the term reproducible is
interpreted in that way?

That's not reproducible in this context [1] (I mean software
reproducibility), or to be unambiguous we should always use "build
reproducible" instead of "reproducible" so we amost recall some context
[2]?

IMVHO if we continue using the term reproducible in that trivial way [3]
when talking about software (this include each and every scientific
paper [4]), we will never get to any point; reproducible is what
reproducible means: https://reproducible-builds.org/docs/definition/

I was just hoping that nowadays "reproducible" is perceived as "build
reproducible" (as defined in the definition above) by all software
developers and many users (including scientists)

... the same holds for "bootstrappable" (in this context)


Happy Hacking!  Gio'

P.S.: or you Maxime are just playng the devil's advocate? :-D



[1] context is what defines the meaning of symbols ;-)

[2] build what?

[3] scan and print (aka copy) and distribute the compiled binary
artifact

[4] is it software even if not always expressed in a programming
language? (my guess is yes)

-- 
Giovanni Biscuolo

Xelera IT Infrastructures

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

Re: On commit access, patch review, and remaining healthy

[Maxime Devos]

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

Giovanni Biscuolo schreef op ma 13-06-2022 om 11:34 [+0200]:
> Maxime I have a question for you please: do you really think that in
> the NixOS community

Going by the Java example, yes, at least for some of the NixOS
community.  I've also seen this interpretation of reproducibility in
Clojure (there was some question on building things from source and
reproducibility, and the response was something along the line ‘using
upstream binaries is 100% reproducible’).

>  (or any other project mentioned in
> https://reproducible-builds.org/who/projects/) the term reproducible
> is
> interpreted in that way?

I don't know about all of them, but for Guix and Debian: no.

> IMVHO if we continue using the term reproducible in that trivial way
> [3]
> when talking about software (this include each and every scientific
> paper [4]), we will never get to any point; reproducible is what
> reproducible means: https://reproducible-builds.org/docs/definition/

Exactly, trivial interpretations aren't really the point, because
trivial.

> [...]
> I was just hoping that nowadays "reproducible" is perceived as "build
> reproducible" (as defined in the definition above) by all software
> developers and many users (including scientists)

I'd hope so, yes.

> P.S.: or you Maxime are just playng the devil's advocate? :-D

No, I'm not advocating that trivial reproducibility is useful or the
goal or such.  My response was some speculation on an answer to the
following question:

> but it's impossible to me to understand how a packaged upstream jar
> can be considered reproducible (and bootstrappable);

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

Re: On commit access, patch review, and remaining healthy

[Arun Isaac]

Hi Giovanni,

Rather than any specific criticism of our workflow, my point was that
there are many little things to keep in mind when reviewing patches, and
this makes it demanding. I am pretty happy with Guix's rigorous coding
standards, and am not suggesting we abandon them.

Cheers! :-)
Arun

Re: On commit access, patch review, and remaining healthy

[Giovanni Biscuolo]

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

Hi Maxime

Maxime Devos <maximedevos@telenet.be> writes:

> Giovanni Biscuolo schreef op ma 13-06-2022 om 11:34 [+0200]:
>> Maxime I have a question for you please: do you really think that in
>> the NixOS community
>
> Going by the Java example, yes, at least for some of the NixOS
> community.  I've also seen this interpretation of reproducibility in
> Clojure (there was some question on building things from source and
> reproducibility, and the response was something along the line ‘using
> upstream binaries is 100% reproducible’).

Ouch!  It hurts so much ;-(

[...]

>> P.S.: or you Maxime are just playng the devil's advocate? :-D
>
> No, I'm not advocating that trivial reproducibility is useful or the
> goal or such.  My response was some speculation on an answer to the
> following question:
>
>> but it's impossible to me to understand how a packaged upstream jar
>> can be considered reproducible (and bootstrappable);

OK OK, now I understand, I did not imagine there's still this kind of
interpretation about reproducible builds in some groups of
developers/maintainers


Thanks!  Gio'

-- 
Giovanni Biscuolo

Xelera IT Infrastructures

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

Re: On commit access, patch review, and remaining healthy

[zimoun]

Hi,

On Sat, 11 Jun 2022 at 01:13, Thiago Jung Bauermann <bauermann@kolabnow.com> wrote:

> But I do think it's one more source of “friction” for new contributors,
> and one more thing for us to require that they get right.

[...]

> There's one in the GNU Coding Standards¹:

[...]

> Personally, I think nowadays this purpose is better fulfilled by
> good commit messages and git blame. Especially with an editor that makes
> it easy to use them to navigate through history (such as Emacs, but
> certainly others as well).

I agree that Emacs+Magit among many others make easy to navigate through
the history.  However, the commit messages are probably good enough
because some Coding Standards are imposed.

Because these standards, it is easy to navigate via grep for instance.
Git blame is useful once you know exactly what you are looking for.
Before that, when I try to figure out the logic behind such change, the
commit messages more or less fixed by the standards are very helpful,
IMHO.

Whatever the style (ChangeLog or anything else), it appears to me a good
thing to have strong standards.


Cheers,
simon

Re: On commit access, patch review, and remaining healthy

[zimoun]

Hi,

On Wed, 08 Jun 2022 at 11:30, Giovanni Biscuolo <g@xelera.eu> wrote:

>> It reduces a bit the pressure on the committers, IMHO.
>
> It raises a bit the pressure on the maintainers, IMHO :-)

What does it mean “maintainer” here?

Maybe I miss something but I do not think the Guix maintainers play a
special role in reviewing or committing.  Could you explain which
pressure you are envisioning?


> I understand there is a certain "entrance barrier" to become patch
> reviewer, but I'm afraid we cannot lower it more than the current
> situation except for the offload build server and more tolling options.

I am missing the meaning of «tolling option».

I think it is possible to lower a bit the reviewing barrier.  Today, the
patch submission is very flexible: it is possible to send inline
patches, attached patches, mix inline and attach, subject can or not
contain ’vN’ and/or X/Y, base-commit is recommended but not mandatory,
etc.  Therefore, it is hard to automate many reviewing steps.

For instance, consider submission #47171 [1].  It was not my first
contribution, it was not the first review by Ricardo, and we both missed
a “guix pull” breakage despite the fact I did “make as-derivation” (and
I am not convinced it is systematically done ;-)).

Another example, when working of Preservation of Guix [2], I noticed
that many packages using git-fetch were not in SWH; which means that
“guix lint” had not been run on these packages.

We could answer more automated tools on infra side, etc. which is the
direction to go.  But we are not there yet and things need to be done
today. :-)  That’s why, I think the project should:

 1. change the default branch of “git push” vs the default branch of
 “guix pull”.

 2. add a bit more of checkers on patch submission easing patch review.
 

1: <https://issues.guix.gnu.org/47171#12>
2: <https://yhetil.org/guix/CAJ3okZ1Jv+ghZFLPhBVWTroamAfss66fkUzHAxorJx5P4Kjz3A@mail.gmail.com/>


Cheers,
simon

Re: On commit access, patch review, and remaining healthy

[Maxim Cournoyer]

Hello,

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

> Hi,
>
> On Sat, 11 Jun 2022 at 01:13, Thiago Jung Bauermann <bauermann@kolabnow.com> wrote:
>
>> But I do think it's one more source of “friction” for new contributors,
>> and one more thing for us to require that they get right.
>
> [...]
>
>> There's one in the GNU Coding Standards¹:
>
> [...]
>
>> Personally, I think nowadays this purpose is better fulfilled by
>> good commit messages and git blame. Especially with an editor that makes
>> it easy to use them to navigate through history (such as Emacs, but
>> certainly others as well).
>
> I agree that Emacs+Magit among many others make easy to navigate through
> the history.  However, the commit messages are probably good enough
> because some Coding Standards are imposed.
>
> Because these standards, it is easy to navigate via grep for instance.
> Git blame is useful once you know exactly what you are looking for.
> Before that, when I try to figure out the logic behind such change, the
> commit messages more or less fixed by the standards are very helpful,
> IMHO.

I agree.  I've come to like GNU ChangeLog commit messages because it
forces me to lay down the changes I've worked on, and sometimes I can
spot things that would be better separated in its own commit, or that
was unintentionally left while testing.

When reviewing others' work it also give me a clear trail of what they
did, and I can match the actual changes to their high level description.

> Whatever the style (ChangeLog or anything else), it appears to me a good
> thing to have strong standards.

Agreed!

Thanks,

Maxim

Re: On commit access, patch review, and remaining healthy

[Arun Isaac]

>>> Personally, I think nowadays this purpose is better fulfilled by
>>> good commit messages and git blame. Especially with an editor that makes
>>> it easy to use them to navigate through history (such as Emacs, but
>>> certainly others as well).
>>
>> Because these standards, it is easy to navigate via grep for instance.
>> Git blame is useful once you know exactly what you are looking for.
>> Before that, when I try to figure out the logic behind such change, the
>> commit messages more or less fixed by the standards are very helpful,
>> IMHO.
>
> I agree.  I've come to like GNU ChangeLog commit messages because it
> forces me to lay down the changes I've worked on, and sometimes I can
> spot things that would be better separated in its own commit, or that
> was unintentionally left while testing.

Exactly! I too have grown to like our ChangeLog style commit messages
enough that I use it in my own repositories.

I like to think of them as akin to the Pointing and Calling[1] system
common on Japanese railways. In the Pointing and Calling system, the
railway worker has to not only go through a checklist of items, but also
has to actually *point* (gesturing) and *call" (verbalizing) the
relevant dial or traffic signal. This explicit gesturing and verbalizing
makes them pay more attention and reduces the probability of an
accident. Likewise, our ChangeLog style commit messages force the
committer to explicitly list down the changes and thus catch any lapses
of concentration.

[1]: https://en.wikipedia.org/wiki/Pointing_and_calling

> When reviewing others' work it also give me a clear trail of what they
> did, and I can match the actual changes to their high level
> description.

I agree! :-)

Re: On commit access, patch review, and remaining healthy

[Arun Isaac]

Hi,

>  That’s why, I think the project should:
>
>  1. change the default branch of “git push” vs the default branch of
>  “guix pull”.
>
>  2. add a bit more of checkers on patch submission easing patch
>  review.

I like and support both these ideas. Maybe, they are even long overdue!
;-)

I would also like to raise a couple of more controversial suggestions:

Should we restrict the set of packages that will be accepted into Guix?
Currently, we accept practically any free software package into
Guix. Should we limit the number of packages we will accept in order to
ease maintenance? "Minimal" distros like Arch Linux do this, for
example.

The cons are that, say if we reject packages involving difficult
languages (think javascript), we may alienate a section of our users
(and potential users) and thus inhibit further growth. If we go down
this route, Guix may never grow into an "universal distribution" like
Debian is.

Also, should we remove old/broken/unused/rarely-used packages from Guix?
In the past, I have packaged and contributed very niche packages which
probably no one else uses, and sometimes even I don't use anymore. But,
these packages continue to stay in Guix and add to the maintenance
burden. Should we have some policy to phase out such packages,
especially if such packages break often? I mean, that there is no need
to phase out an elisp package that builds trivially all the time, but
what about more complex packages that take many many hours to maintain?

I don't have strong opinions on these questions. I would love to hear
what others think.

Cheers,
Arun

Re: On commit access, patch review, and remaining healthy

[Ludovic Courtès]

Hi,

Arun Isaac <arunisaac@systemreboot.net> skribis:

>>  That’s why, I think the project should:
>>
>>  1. change the default branch of “git push” vs the default branch of
>>  “guix pull”.
>>
>>  2. add a bit more of checkers on patch submission easing patch
>>  review.
>
> I like and support both these ideas. Maybe, they are even long overdue!
> ;-)

I like them too!

#1 is more involved than it sounds though: there would need some tooling
and/or human intervention to merge things from the push branch to the
pull branch.  I don’t have a clear idea of how to do this.

> I would also like to raise a couple of more controversial suggestions:
>
> Should we restrict the set of packages that will be accepted into Guix?
> Currently, we accept practically any free software package into
> Guix. Should we limit the number of packages we will accept in order to
> ease maintenance? "Minimal" distros like Arch Linux do this, for
> example.
>
> The cons are that, say if we reject packages involving difficult
> languages (think javascript), we may alienate a section of our users
> (and potential users) and thus inhibit further growth. If we go down
> this route, Guix may never grow into an "universal distribution" like
> Debian is.

I’ve been wondering about that too.  Clearly there’s a tension here.
The main difficulty as I see it is how do we draw a line?

For example, assume we came up two years ago with a policy to not
include Rust packages and instead have them in a guix-rust channel.  Two
years later, Rust has become a dependency of a number of core packages,
so we have to have Rust, or at least a large subset thereof, in the main
channel.

Then there’s the question of leaf packages (applications): how do you
assess the usefulness of an application?  How do you determine that an
application is no longer used?

> Also, should we remove old/broken/unused/rarely-used packages from Guix?
> In the past, I have packaged and contributed very niche packages which
> probably no one else uses, and sometimes even I don't use anymore. But,
> these packages continue to stay in Guix and add to the maintenance
> burden. Should we have some policy to phase out such packages,
> especially if such packages break often? I mean, that there is no need
> to phase out an elisp package that builds trivially all the time, but
> what about more complex packages that take many many hours to maintain?

I think we do that from time to time, but it’s an entirely manual
process.  Sometimes a patch to remove a package triggers a reply with a
patch to fix that package’s build process too (I’ve done that a couple
of times :-)).

What could help though is a dashboard, in Cuirass or in the Data
Service, that would display packages that have been failing to build
“for some time”.

Thoughts,
Ludo’.

Re: On commit access, patch review, and remaining healthy

[Giovanni Biscuolo]

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

Hi Simon,

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

> On Wed, 08 Jun 2022 at 11:30, Giovanni Biscuolo <g@xelera.eu> wrote:
>
>>> It reduces a bit the pressure on the committers, IMHO.
>>
>> It raises a bit the pressure on the maintainers, IMHO :-)
>
> What does it mean “maintainer” here?

Guix maintainers

> Maybe I miss something but I do not think the Guix maintainers play a
> special role in reviewing or committing.

not directly but they oversee the entire process, no?

> Could you explain which pressure you are envisioning?

it was explained above your quotation in my original message, IMHO this
is the pressure:

--8<---------------cut here---------------start------------->8---

> **automatically** merged every week to the branch “stable” and by
> default user pull “stable”.  One week let the time to build by the CI,
> check everything is fine and fix otherwise.

This means that if the fix is not committed (rebased?) in that weekly
timerfame the problematic patch is automatically pushed to stable
without a fix; also we'll have that problematic commit in stable anyway
(affecting users like me that are "pinning" specific channels?), unless
we rebase "unstable"... "manually": am I wrong?

--8<---------------cut here---------------end--------------->8---

IMVHO automatic merges once a week from something /possibly/ not working
to "stable" is not a good solution to the problem of reviwers scarcity

>> I understand there is a certain "entrance barrier" to become patch
>> reviewer, but I'm afraid we cannot lower it more than the current
>> situation except for the offload build server and more tolling options.
>
> I am missing the meaning of «tolling option».

sorry, my intention was to write "tooling options", meaning the range of
tools available to committers/reviewers to automate some tasks

> I think it is possible to lower a bit the reviewing barrier.  Today, the
> patch submission is very flexible:

IMHO this is a good thing, it lowers the barrier for new contributors

[...]

> For instance, consider submission #47171 [1].

seen

> It was not my first contribution, it was not the first review by
> Ricardo, and we both missed a “guix pull” breakage despite the fact I
> did “make as-derivation” (and I am not convinced it is systematically
> done ;-)).

as Ludo' was suggesting, maybe we could start with a checklist and then
see what we can automate?

> Another example, when working of Preservation of Guix [2], I noticed
> that many packages using git-fetch were not in SWH; which means that
> “guix lint” had not been run on these packages.

is there any way to force it (along with other linting) when commiting?

> We could answer more automated tools on infra side, etc. which is the
> direction to go.  But we are not there yet and things need to be done
> today. :-)  That’s why, I think the project should:
>
>  1. change the default branch of “git push” vs the default branch of
>  “guix pull”.

sorry I don't understand what this means

>  2. add a bit more of checkers on patch submission easing patch
>  review.

I guess you mean "automatic checkers": I agree that checking tools
are good (something is missing in "guix lint"?)

[...]

Thank you! Gio'

-- 
Giovanni Biscuolo

Xelera IT Infrastructures

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

Re: On commit access, patch review, and remaining healthy

[Paul Jewell]

> On 15 Jun 2022, at 09:15, Arun Isaac <arunisaac@systemreboot.net> wrote:
> 
> I would also like to raise a couple of more controversial suggestions:
> 
> Should we restrict the set of packages that will be accepted into Guix?
> Currently, we accept practically any free software package into
> Guix. Should we limit the number of packages we will accept in order to
> ease maintenance? "Minimal" distros like Arch Linux do this, for
> example.
> 
> The cons are that, say if we reject packages involving difficult
> languages (think javascript), we may alienate a section of our users
> (and potential users) and thus inhibit further growth. If we go down
> this route, Guix may never grow into an "universal distribution" like
> Debian is.
> 

If there is someone willing to maintain the packages, then in my opinion such restrictions shouldn’t be applied. I guess this would mean knowing who is the maintainer for each package in guix, but this could also be a team (reference the recent teams discussion).

> Also, should we remove old/broken/unused/rarely-used packages from Guix?
> In the past, I have packaged and contributed very niche packages which
> probably no one else uses, and sometimes even I don't use anymore. But,
> these packages continue to stay in Guix and add to the maintenance
> burden. Should we have some policy to phase out such packages,
> especially if such packages break often? I mean, that there is no need
> to phase out an elisp package that builds trivially all the time, but
> what about more complex packages that take many many hours to maintain?

I think they should be removed. This could link to the maintainer comment above. If a package is identified as old or broken, then users could be notified, and after a “cooling off” period they are removed. This could allow for discussion about whether removal is appropriate, or whether someone else would step up and update the package. Under Gentoo (the distro I know well, having used it since 2004), obsolete and problematic packages are hard masked, and users notified why, and when removal from the repository will occur. The hard masking prevents new installations (almost - you can make some additional configuration changes to enable installation). Users then have a choice - support the resolution of the issues leading to hard masking, or move the package definition to a personal repository so it can continue to be used (at their own risk). Regarding rarely used packages - I wouldn’t remove these if there is a maintainer for them and they are still being kept up to date. If this no longer happens, then they are in the old/broken category.

I guess the big question for me is “could this be automated in some way?”

> I don't have strong opinions on these questions. I would love to hear
> what others think.
> 

I hope my comments are useful!

Best regards,
Paul

Re: On commit access, patch review, and remaining healthy

[Arun Isaac]

>> Also, should we remove old/broken/unused/rarely-used packages from Guix?
>> In the past, I have packaged and contributed very niche packages which
>> probably no one else uses, and sometimes even I don't use anymore. But,
>> these packages continue to stay in Guix and add to the maintenance
>> burden. Should we have some policy to phase out such packages,
>> especially if such packages break often? I mean, that there is no need
>> to phase out an elisp package that builds trivially all the time, but
>> what about more complex packages that take many many hours to maintain?
>
> I think they should be removed. This could link to the maintainer
> comment above. If a package is identified as old or broken, then users
> could be notified, and after a “cooling off” period they are
> removed. This could allow for discussion about whether removal is
> appropriate, or whether someone else would step up and update the
> package. Under Gentoo (the distro I know well, having used it since
> 2004), obsolete and problematic packages are hard masked, and users
> notified why, and when removal from the repository will occur. The
> hard masking prevents new installations (almost - you can make some
> additional configuration changes to enable installation). Users then
> have a choice - support the resolution of the issues leading to hard
> masking, or move the package definition to a personal repository so it
> can continue to be used (at their own risk). Regarding rarely used
> packages - I wouldn’t remove these if there is a maintainer for them
> and they are still being kept up to date. If this no longer happens,
> then they are in the old/broken category.

I like this idea. We already have a way to mark packages as
deprecated. The Gentoo method seems like a way to mark packages that are
"about to be deprecated". I like the way it notifies the user of the
impending deprecation without the user having to subscribe to the
mailing list or the issue tracker.

> I guess the big question for me is “could this be automated in some
> way?”

Indeed, it's all a matter of somebody implementing the tooling! ;-) Like
Ludo mentioned, Cuirass and the Guix data service may be involved in
determining which packages are frequently broken.

>> I don't have strong opinions on these questions. I would love to hear
>> what others think.
>> 
>
> I hope my comments are useful!

Definitely, thanks!

Re: On commit access, patch review, and remaining healthy

[Hartmut Goebel]

Hi,

here are my reasons for reviewing patches very very rarely-

Basically I share Brian Cully's experiences. I'm using Thunderbird for 
mail and my system is set up to emacs could send out mails.

I tried debbugs from time to time and for me it is disgusting:

  * too complicated to get to the list of patches or bugs ( I never can
    remember the many key-presses to perform) ,
  * I did not manage to apply patches from there (emacs would need to
    know where my guix development directory is - how can I tell it?)
  * commands withing debbugs feel complicated
  * if a ticket contains several updated patches, its very hard to find
    those relevant (one of the reasons of forges' success is that they
    present you the current state)
  * actually testing the patches required to apply the patches to my
    worktree - and chances are high 'git am' will fail with some
    conflict - chances raise extremely for old patches
  * Over all for me debbugs.el needs a much more "noops"-friendly interface

Regarding the actual review:

  * Yes, I miss a review guide-line
  * As Arun wrote: Guix has high quality standards. I feel uncomfortable
    with judging whether a summary or description is good enough. Also
    I'm not a native speaker and don't feel entitled to review English
    gramar and spelling.
  * I miss a way to contribute to a review but not actually approving
    it. (In git(lab,hub) I could add comments other reviewers and the
    submitter could resolve or give feedback. This allows me to focus on
    e.g. some parts, while someone else could review the summary and
    description.)
  * I also miss automated tests. E.g. it dos not make sense to waste my
    time for running 'guix lint', as a automate could do this.

When agreeing to a patch:

  * I'd like to simply merge the patch without taking care abut whether
    the submitter has commit right. This saves time for the submitter.
    Sending "LGTM" is pleasant, anyhow wasting time. The reviewer needs
    to send a mail, the submitter needs to dig the branch, rebase it and
    push. If the reviewer already did merge it, he/she could push, too,
    IMHO.
  * And for me as a submitter: I want my patches to be merged by the
    reviewer.


Am 07.06.22 um 17:11 schrieb Ludovic Courtès:
> Do you or would you use them to keep track of pending patches?

I use issues.guix.gnu.org eventually. Anyhow this is browse-only. I did 
not even manage to add a reply there.

-- 
Regards
Hartmut Goebel

| Hartmut Goebel          |h.goebel@crazy-compilers.com                |
|www.crazy-compilers.com  | compilers which you thought are impossible |

Re: On commit access, patch review, and remaining healthy

[zimoun]

Hi Hartmut,

On Mon, 20 Jun 2022 at 14:53, Hartmut Goebel <h.goebel@crazy-compilers.com> wrote:

>          my system is set up to emacs could send out mails.

Well, if you are already using Emacs, the Emacs front-end is not the
nicest interface but does part of the job.  I have this:

--8<---------------cut here---------------start------------->8---
(setq
   debbugs-gnu-default-packages '("guix-patches")
   gnus-summary-line-format "%I%(%[ %n%]%) %s\n")
--8<---------------cut here---------------end--------------->8---

Then ’M-x debbugs-gnu’.  Yeah, I agree it is a bit slow. :-)


> I tried debbugs from time to time and for me it is disgusting:
>
>   * too complicated to get to the list of patches or bugs ( I never can
>     remember the many key-presses to perform) ,

To me, the annoyance comes from that ’M-x debbugs-gnu-bugs RET’ returns
the 10 last submissions but it is for the whole GNU project; when I
would like to filter only ’guix-patches’ or ’guix’.


>   * I did not manage to apply patches from there (emacs would need to
>     know where my guix development directory is - how can I tell it?)
>   * commands withing debbugs feel complicated
>   * if a ticket contains several updated patches, its very hard to find
>     those relevant (one of the reasons of forges' success is that they
>     present you the current state)

Well, submitter should use ’git format-patch --reroll-count’. )-:

>   * actually testing the patches required to apply the patches to my
>     worktree - and chances are high 'git am' will fail with some
>     conflict - chances raise extremely for old patches

Again, submitter should use ’git format-patch --base’; as specified by
the manual. :-)  Using this ’base-commit’, you know where the patch
applies and so you can checkout against the right correct.


>   * Over all for me debbugs.el needs a much more "noops"-friendly
>   interface

Well, I think ’public-inbox’ could help.  An instance is:

    https://yhetil.org/guix-patches/

where using lei, you can filter and receive to your inbox the patches.
For example, ’lei’ allows to filter (and thus follow), e.g., let query
for the patches submitted the last 3 months to
’gnu/packages/bioinformatics.scm’ touching R packages,

dfn:gnu/packages/bioinformatics AND b:r-build-system AND rt:3.months.ago..

where

   dfn:     match filename from diff
   b:       match within message body, including text attachments
   rt:      match date-time range, git "approxidate" formats supported
            Open-ended ranges such as `d:last.week..' and `d:..2.days.ago'
            are supported

<https://yhetil.org/guix/?q=dfn%3Agnu%2Fpackages%2Fbioinformatics+AND+b%3Ar-build-system+AND+rt%3A3.months.ago..>

Then, ’lei up’ will request the public-inbox server for this query and
will download the messages if any.  Therefore, you can filter and
receive only the patches that interest you.  Then you could save such
patches and apply them using ’git am’.




Cheers,
simon

Re: On commit access, patch review, and remaining healthy

[Munyoki Kilyungi]

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

[...]

>>   * Over all for me debbugs.el needs a much more "noops"-friendly
>>   interface
>
> Well, I think ’public-inbox’ could help.  An instance is:
>
>     https://yhetil.org/guix-patches/
>
> where using lei, you can filter and receive to your inbox the patches.
> For example, ’lei’ allows to filter (and thus follow), e.g., let query
> for the patches submitted the last 3 months to
> ’gnu/packages/bioinformatics.scm’ touching R packages,
>

Another less known alternative that may be useful
is to set up a local NNTP server (like say
LeafNode[0]), and subscribe to patches on GNUs (or
any other newsreader interface you find
comfortable).  That's what I've done with
mailing-lists.  I'm yet to work out how to
incorporate NotMuch for a better search
experience.

[0] https://www.techrepublic.com/article/news-with-leafnode/

[...]

<#secure method=pgpmime mode=sign>

-- 
Munyoki Kilyungi
D4F09EB110177E03C28E2FE1F5BBAE1E0392253F (hkp://keys.gnupg.net)
Free Software Activist
Humble GNU Emacs User | Bearer of scheme-y parens