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

[Pier-Hugues Pellerin <ph@heykimo.com>]

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