Patch queue management systems

Patch queue management systems

[Eli Zaretskii]

[Moved the discussion to emacs-devel.]

> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sat, 06 Dec 2014 04:27:22 +0200
> 
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> 
> > Because nobody has set such a thing up (and AFAICT Savannah doesn't
> > have such a thing built-in, oddly enough).
> 
> It shouldn't be too hard, actually, to set up some code review tool,
> such as Gerrit [0], Review Board [1] or Differential [2].

I took a quick look at these.

I think by far the 2 main issues we will have with these tools are:

 . The tools don't have Emacs integration built into them, nor offer
   Emacs VC sub-modes.  (Gerrit has a magit plugin.)  I'm guessing we
   would like to arrange for such an integration in VC, because people
   who review patches are accustomed to using Emacs and have elaborate
   customizations for this that they would lose if the review is only
   done via a Web browser (even if that browser is eww).

 . The tools require non-trivial changes in the workflow advertised on
   the Wiki.  E.g., Gerrit requires to work with magic local branches
   and detached heads.  We should carefully evaluate this and decide
   how to make sure this won't trip those of us who are less
   proficient in Git (which, judging by the current traffic on
   emacs-devel, seems like a rule rather than exception) and cause
   corruption of the upstream repository.

There are also other minor issues we need to figure out: licenses, the
underlying infrastructure (e.g., AFIU, Phabricator requires PHP), etc.

Personally, I think arranging the development around this kind of
process will not work without some critical mass of patch reviewers
who are able to endure the current constant high volume of changes,
let alone if we want to increase that volume.  Being an efficient
patch reviewer requires good knowledge of at least a few areas of the
Emacs core, and we currently have only a handful of people who can
qualify.  (The obvious exception from this rule is a maintainer of a
single package who can review patches for his/her package.)  From
experience of other projects, 5 reviewers is not enough for this task.

So we are again back to the hardest problem in Emacs development: the
extremely small number of core maintainers.

> I'd also welcome opinions from people who have experience working with
> them

Seconded.

Re: Patch queue management systems

[Ted Zlatanov]

EZ> Personally, I think arranging the development around this kind of
EZ> process will not work without some critical mass of patch reviewers
EZ> who are able to endure the current constant high volume of changes,
EZ> let alone if we want to increase that volume.  Being an efficient
EZ> patch reviewer requires good knowledge of at least a few areas of the
EZ> Emacs core, and we currently have only a handful of people who can
EZ> qualify.  (The obvious exception from this rule is a maintainer of a
EZ> single package who can review patches for his/her package.)  From
EZ> experience of other projects, 5 reviewers is not enough for this task.

I wouldn't mind reviewing things in the queue when I'm able.

My wishlist for a patch queue management system (PR/pull request system
for those familiar with the Github model):

* targeted hydra builds for a specific branch

* a click-and-done way to merge a specific branch into master or emacs-24

* maybe a comment system

* maybe debbugs integration (but I do hate debbugs... really hate it...)

* ability to review the potential ChangeLog entries that will be
  generated from the commit

All of these, put together, would make it a pleasure to use.  As a
significant side benefit, Emacs wouldn't be the only project to benefit
from these improvements if they were made in Savannah.

Ted

Re: Patch queue management systems

[Thomas Koch]

We use gerrit at work so I can comment for it.

On Saturday, December 06, 2014 08:12:31 AM Ted Zlatanov wrote:
> My wishlist for a patch queue management system (PR/pull request system
> for those familiar with the Github model):
> 
> * targeted hydra builds for a specific branch
I don't know hydra. We use jenkins which has a very good plugin to integrate 
with gerrit. Every commit that lands in gerrit triggers a jenkins build and 
jenkins leaves a vote afterwards in gerrit. Thus a human doesn't need to do a 
review as long as the jenkins build isn't good.

> * a click-and-done way to merge a specific branch into master or emacs-24
This is called 'submit' in gerrit and it's click-and-done.

> * maybe a comment system
In gerrit you can comment and reply on every line of the commit, the commit 
message or comment on the whole change.

> * maybe debbugs integration (but I do hate debbugs... really hate it...)
- easy: link from gerrit changes to debbugs if a bug is referenced in the 
commit message
- a bit more involved: trigger debbugs actions for actions in gerrit (new 
change, change merged...)

> * ability to review the potential ChangeLog entries that will be
>   generated from the commit
The commit message is part of the review. We generate changelogs from commit 
messages and add an 'ignore:' trailer if a commit should not be included in 
the changelog.

A massive advantage of gerrit over reviewboard as used by the Apache 
Foundation is the frictionless integration with git.

On the other hand gerrit is extremely powerful and customizable which might be 
a problem for somebody setting it up for the first time.

Regards, Thomas Koch

Re: Patch queue management systems

[Lars Magne Ingebrigtsen]

Ted Zlatanov <tzz@lifelogs.com> writes:

> My wishlist for a patch queue management system (PR/pull request system
> for those familiar with the Github model):
>
> * targeted hydra builds for a specific branch
>
> * a click-and-done way to merge a specific branch into master or emacs-24
>
> * maybe a comment system
>
> * maybe debbugs integration (but I do hate debbugs... really hate it...)

I like debbugs better than any other bug tracking system I've used.
Because it's in Emacs.  :-)  `/ tls' and there all the TLS-related bugs
are.

But it does need more features to make it possible to type less when
applying patches.  A one-keystroke command to apply the patch, put the
bug number into the ChangeLog, and do a "make" in a buffer to see
whether any compilation warnings are generated, as well as a one-touch
"make the commit message and check this stuff in" command.

So, a one-key solution to apply and test a patch, and a one-key solution
to checking it in.

It's doable if somebody feels like doing it.

As for tracking appliable patches, debbugs already offers that with the
"patch" and "pending" tags.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Patch queue management systems

[Lars Magne Ingebrigtsen]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> It's doable if somebody feels like doing it.

Yeah, yeah.

One of the most annoying things about applying patches is looking around
for whether the person has assigned copyright to the FSF?  Is there a
reason why this list isn't available as a...  web service?  Or something?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Patch queue management systems

[Lars Magne Ingebrigtsen]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> One of the most annoying things about applying patches is looking around
> for whether the person has assigned copyright to the FSF?  Is there a
> reason why this list isn't available as a...  web service?  Or something?

I just wrote a function to grep changelogs for names.  Works almost as
well.  :-)

Perhaps debbugs should have an automatic "nag the maintainer"
functionality.  I mean, at least once.  There's a lot of bug reports in
there that I think are very difficult for a third party to evaluate
whether make sense or not.

Of course, large swathes of Emacs are unmaintained...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Patch queue management systems

[Stefan Monnier]

> It's doable if somebody feels like doing it.

I'd love it,


        Stefan

Re: Patch queue management systems

[Lars Magne Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> It's doable if somebody feels like doing it.
>
> I'd love it,

Great, because I've started implementing it.  :-)  But I'm not promising
this in a timely fashion...  Lots of fiddly systems interacting with
each other.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Patch queue management systems

[Dmitry Gutov]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> I like debbugs better than any other bug tracking system I've used.
> Because it's in Emacs.  :-)  `/ tls' and there all the TLS-related bugs
> are.

A lot of people dislike it, myself included. There was a yet-another
thread about a replacement recently, but apparently it fizzled out. Too
bad.

> But it does need more features to make it possible to type less when
> applying patches.  A one-keystroke command to apply the patch, put the
> bug number into the ChangeLog, and do a "make" in a buffer to see
> whether any compilation warnings are generated, as well as a one-touch
> "make the commit message and check this stuff in" command.

This still won't compare with a decent code review system:

- Externally visible queue of contributions, so that people from outside
  can get a better look at Emacs development, and maybe learn something.
  And speaking of debbugs, I can even get a list of "all bugs with
  patches" from inside of Emacs.

- A working integration server that gives an overview of whether each
  patch (or better: Git branch) compiles, passes tests, etc, which will
  be a good reminder for a contributor if they broke something, even
  before a reviewer is involved.

- Inline comments: hide the old ones as the discussion progresses, only
  show the ones still pertinent to the current changeset (though this
  part may be hard to integrate into Emacs either way). I was kinda
  hoping it might would serve as an alternative for debbugs comments.

- If Git branches are involved, and ChangeLogs are phased out, an
  automatic "merge this branch" action would be natural to use, instead
  of doing the things you've enumerated above. If course, that's two
  whole ifs.

Re: Patch queue management systems

[Lars Magne Ingebrigtsen]

Dmitry Gutov <dgutov@yandex.ru> writes:

> This still won't compare with a decent code review system:
>
> - Externally visible queue of contributions, so that people from outside
>   can get a better look at Emacs development, and maybe learn something.
>   And speaking of debbugs, I can even get a list of "all bugs with
>   patches" from inside of Emacs.

Sure you can.  `C-u / patch' in debbugs-gnu.

> - A working integration server that gives an overview of whether each
>   patch (or better: Git branch) compiles, passes tests, etc, which will
>   be a good reminder for a contributor if they broke something, even
>   before a reviewer is involved.

This can all be done by adding tags to bug reports in debbugs.  

> - Inline comments: hide the old ones as the discussion progresses, only
>   show the ones still pertinent to the current changeset (though this
>   part may be hard to integrate into Emacs either way). I was kinda
>   hoping it might would serve as an alternative for debbugs comments.

That sounds nice, but 99% of patches in the bug tracker are small and
need only a couple passes or less before they can be applied.  So this
would give us no extra patch handling power in reality.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Patch queue management systems

[Dmitry Gutov]

On 12/09/2014 07:20 PM, Lars Magne Ingebrigtsen wrote:

>> I can['t] even get a list of "all bugs with patches" from inside of Emacs.
>
> Sure you can.  `C-u / patch' in debbugs-gnu.

Okay, thanks. Any reason why it includes closed bugs in the list? And 
could Debbugs be made to work snappier? 20000 is not much, on a database 
scale.

>> - A working integration server that gives an overview of whether each
>>    patch (or better: Git branch) compiles, passes tests, etc, which will
>>    be a good reminder for a contributor if they broke something, even
>>    before a reviewer is involved.
>
> This can all be done by adding tags to bug reports in debbugs.

...I suppose. Note that integration server build status usually includes 
a status value (passed, failed, or errored before it had a chance to 
fail) and a url to the build info (where we can look at the log).

Do you intend to keep the patch submission workflow the same? I.e. 
free-form (inline or in an attachment with arbitrary name), instead of a 
Git branch somewhere. How would integration server pick up and apply 
such patches?

>> - Inline comments: hide the old ones as the discussion progresses, only
>>    show the ones still pertinent to the current changeset (though this
>>    part may be hard to integrate into Emacs either way). I was kinda
>>    hoping it might would serve as an alternative for debbugs comments.
>
> That sounds nice, but 99% of patches in the bug tracker are small and
> need only a couple passes or less before they can be applied.  So this
> would give us no extra patch handling power in reality.

It increases visibility into the discussion. Even if we're only talking 
two passes, being able to see just the comments that apply to the latest 
proposed version is pretty nice.

Though of course, the smaller patches that don't need discussion would 
benefit more from being able to see that the build passes and instantly 
merge the contribution.

Re: Patch queue management systems

[Lars Magne Ingebrigtsen]

Dmitry Gutov <dgutov@yandex.ru> writes:

> Okay, thanks. Any reason why it includes closed bugs in the list?

They're included for a few weeks.  `x' to make them go away.

> And could Debbugs be made to work snappier? 20000 is not much, on a
> database scale.

Indeed.

> ...I suppose. Note that integration server build status usually
> includes a status value (passed, failed, or errored before it had a
> chance to fail) and a url to the build info (where we can look at the
> log).

Well, that's also nice, but for virtually all patches we get (which are
to the Lisp bits), running a "make" in the current tree is sufficient to
see whether the patch is whack or not.  No need to wait for a result
from an integration server.

("make test" if you want to be all modern and hang with the cool kids.)

> Do you intend to keep the patch submission workflow the same?
> I.e. free-form (inline or in an attachment with arbitrary name),
> instead of a Git branch somewhere.

Sending patches is really easy, even for kids these days.  

> It increases visibility into the discussion. Even if we're only
> talking two passes, being able to see just the comments that apply to
> the latest proposed version is pretty nice.

Yes, it would be, but we're displaying the discussion threaded, so that
kinda happens naturally.  It would be nice to have a way to mark exactly
what message contains the most current version of the patch, though.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Patch queue management systems

[Dmitry Gutov]

On 12/09/2014 10:44 PM, Lars Magne Ingebrigtsen wrote:

> They're included for a few weeks.  `x' to make them go away.

Thanks.

> Well, that's also nice, but for virtually all patches we get (which are
> to the Lisp bits), running a "make" in the current tree is sufficient to
> see whether the patch is whack or not.  No need to wait for a result
> from an integration server.

Quite the opposite, many bugs with Lisp code can only be found at 
runtime. One has to try hard to break the compilation.

Anyway, you previous message gave me an impression that you do want an 
integration server. You don't? Okay...

> ("make test" if you want to be all modern and hang with the cool kids.)

Right.

>> Do you intend to keep the patch submission workflow the same?
>> I.e. free-form (inline or in an attachment with arbitrary name),
>> instead of a Git branch somewhere.
>
> Sending patches is really easy, even for kids these days.

This ignores the point I was making. To make it easier for automated 
tools, you'd have to make sending patches *harder*, at least a bit, for 
the uninitiated.

>> It increases visibility into the discussion. Even if we're only
>> talking two passes, being able to see just the comments that apply to
>> the latest proposed version is pretty nice.
>
> Yes, it would be, but we're displaying the discussion threaded, so that
> kinda happens naturally.

That's not ideal: only one message's contents is displayed at the time, 
and when reading, one has to go up and down the thread, until they find 
the message that submits the patch, and then switch between the messages 
in the subthread which appear to respond to it. There's no other way to 
follow the discussions on each specific part of the patch, except to 
look at the quotes in each message and try to keep it together in your head.

With inline comments, you can have granular comment threads that only 
apply to specific part of the patch.

> It would be nice to have a way to mark exactly
> what message contains the most current version of the patch, though.

Even that would be an improvement, yes.

Re: Patch queue management systems

[Lars Magne Ingebrigtsen]

Dmitry Gutov <dgutov@yandex.ru> writes:

> Anyway, you previous message gave me an impression that you do want an
> integration server. You don't? Okay...

I do, but it doesn't have to be involved in the patch application
process.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Patch queue management systems

[Lars Magne Ingebrigtsen]

I pushed a DWIM thing to debbugs-gnu that cuts down on patch handling a
bit.  It's easier to explain with pictures:

http://lars.ingebrigtsen.no/2014/12/09/one-touch-maintainin/

It's not quite finished -- there are probably lots of cases where it
doesn't work right.  So fix anything you like.  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Patch queue management systems

[Ted Zlatanov]

On Tue, 09 Dec 2014 22:57:23 +0100 Lars Magne Ingebrigtsen <larsi@gnus.org> wrote: 

LMI> I pushed a DWIM thing to debbugs-gnu that cuts down on patch handling a
LMI> bit.  It's easier to explain with pictures:

LMI> http://lars.ingebrigtsen.no/2014/12/09/one-touch-maintainin/

LMI> It's not quite finished -- there are probably lots of cases where it
LMI> doesn't work right.  So fix anything you like.  :-)

I like the idea.  Very similar to your gmane admin flow :)

I'll try it out.

Ted

Re: Patch queue management systems

[Lars Magne Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Personally, I think arranging the development around this kind of
> process will not work without some critical mass of patch reviewers
> who are able to endure the current constant high volume of changes,
> let alone if we want to increase that volume.

Perhaps we could attract more people by gamifying the Emacs development,
a la Github.  That is, score people on bug closures and stuff.

The winner each month could get a free copy of the Emacs sources.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Patch queue management systems

[joakim]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> Personally, I think arranging the development around this kind of
>> process will not work without some critical mass of patch reviewers
>> who are able to endure the current constant high volume of changes,
>> let alone if we want to increase that volume.
>
> Perhaps we could attract more people by gamifying the Emacs development,
> a la Github.  That is, score people on bug closures and stuff.
>
> The winner each month could get a free copy of the Emacs sources.

I actually have a emacs gamification mode ive been meaning to upload
somewhere. It is very simple yet effective.

(define-minor-mode zen-reward-mode
  "type a lot and get a reward!."
  :global t
  :init-value nil
  :lighter nil
  :keymap nil
  :group 'zen

  (if zen-reward-mode
      (progn (add-hook 'pre-command-hook 'zen-reward-pre-command-hook)
             (add-hook 'org-after-todo-state-change-hook 'zen-reward-todo-state-change-hook))
    (progn (remove-hook 'pre-command-hook 'zen-reward-pre-command-hook)
           (remove-hook 'org-after-todo-state-change-hook 'zen-reward-todo-state-change-hook))))

(defvar  zen-reward-accumulator 0)

(defun zen-reward-todo-state-change-hook ()
  (message "todo hook %s %s" org-state (org-show-priority))
  ;;if you toggle a task back and forth you get super rich.
  ;;you get rewarded for CANCELLED also, but that is good!
  (if  (equal org-state "DONE")
      (progn
        (let ((reward (string-to-number (substring (org-show-priority) (length "Priority is ") ))))
          ;;priority A is 4001, B 3001 and so on. probably can be negative as well. so reward needs to be modified a bit
          (message "todo reward %d!" reward)
          (setq zen-reward-accumulator (+ reward zen-reward-accumulator) ))
))
  )
;testing. only get reward for typing, not for cursor movement and other things
(defun zen-reward-pre-command-hook ()
  "accumulate typing reward."
  (if (or (eq real-last-command 'self-insert-command)
          (eq real-last-command 'org-self-insert-command))
      (setq zen-reward-accumulator (1+ zen-reward-accumulator) ))
;;  (message "reward:%d %s %d" zen-reward-accumulator real-last-command (random))
)  
(defun zen-claim-reward ()
  "cash in your bountiful reward, spend it on fun and games!"
  (interactive )
  (message "reward:%d" zen-reward-accumulator)
  (setq zen-reward-accumulator 0))
(defun zen-inspect-reward ()
  "how soon can i afford fun and games?"
  (interactive )
  (message "zen reward:%d" zen-reward-accumulator)
)

;;
(zen-reward-mode)



-- 
Joakim Verona

Re: Patch queue management systems

[Ivan Andrus]

On Dec 8, 2014, at 3:54 PM, joakim@verona.se wrote:
> 
> Lars Magne Ingebrigtsen <larsi@gnus.org> writes:
> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>>> Personally, I think arranging the development around this kind of
>>> process will not work without some critical mass of patch reviewers
>>> who are able to endure the current constant high volume of changes,
>>> let alone if we want to increase that volume.
>> 
>> Perhaps we could attract more people by gamifying the Emacs development,
>> a la Github.  That is, score people on bug closures and stuff.
>> 
>> The winner each month could get a free copy of the Emacs sources.
> 
> I actually have a emacs gamification mode ive been meaning to upload
> somewhere. It is very simple yet effective.

My gamification package (achievements) is available on MELPA, or at 

https://bitbucket.org/gvol/emacs-achievements

Though I have to admit that I don’t have any achievements for reviewing patches.  Patches welcome. :-)

-Ivan

Re: Patch queue management systems

[Thien-Thi Nguyen]

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

() Lars Magne Ingebrigtsen <larsi@gnus.org>
() Mon, 08 Dec 2014 23:24:44 +0100

   > Personally, I think arranging the development around this
   > kind of process will not work without some critical mass of
   > patch reviewers who are able to endure the current constant
   > high volume of changes, let alone if we want to increase that
   > volume.

   Perhaps we could attract more people by gamifying the Emacs
   development, a la Github.  That is, score people on bug
   closures and stuff.

A (psychologically) related maneuver is specialization (a la
Brooks' MMM "team" concept).  For example, for LibreDWG, i limit
the damage i do by relegating myself to a fussy-bureaucrat role:

 http://lists.gnu.org/archive/html/libredwg/2014-12/msg00012.html

Pignolo, io?!  Ma va... è un dono!  :-D

-- 
Thien-Thi Nguyen
   GPG key: 4C807502
   (if you're human and you know it)
      read my lisp: (responsep (questions 'technical)
                               (not (via 'mailing-list)))
                     => nil

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

Re: Patch queue management systems

[Stefan Monnier]

> The winner each month could get a free copy of the Emacs sources.

More seriously, I'd be happy to send the winner a quart of my home made
ice-cream.


        Stefan

Re: Patch queue management systems

[Lars Magne Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> The winner each month could get a free copy of the Emacs sources.
>
> More seriously, I'd be happy to send the winner a quart of my home made
> ice-cream.

Even more seriously, I could design and make some t-shirts and mail them
to the winners.  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Patch queue management systems

[Stephen Leake]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> Personally, I think arranging the development around this kind of
>> process will not work without some critical mass of patch reviewers
>> who are able to endure the current constant high volume of changes,
>> let alone if we want to increase that volume.
>
> Perhaps we could attract more people by gamifying the Emacs development,
> a la Github.  That is, score people on bug closures and stuff.

On monotone, we had occasional "bugathons".

Somebody would put up $20 or so for the prize of monotone swag (hats,
mugs etc with the monotone logo).

It worked pretty well.

-- 
-- Stephe

Re: Patch queue management systems

[Dmitry Gutov]

On 12/06/2014 12:08 PM, Eli Zaretskii wrote:

> I think by far the 2 main issues we will have with these tools are:
>
>   . The tools don't have Emacs integration built into them, nor offer
>     Emacs VC sub-modes.  (Gerrit has a magit plugin.)  I'm guessing we
>     would like to arrange for such an integration in VC, because people
>     who review patches are accustomed to using Emacs and have elaborate
>     customizations for this that they would lose if the review is only
>     done via a Web browser (even if that browser is eww).

Looking at magit-gerrit, it's not that big. If you're okay with that 
level of integration, it shouldn't take too much work, as long as the 
review system comes with a command-line tool or an API (which, of 
course, will be a requirement).

>   . The tools require non-trivial changes in the workflow advertised on
>     the Wiki.  E.g., Gerrit requires to work with magic local branches
>     and detached heads.  We should carefully evaluate this and decide
>     how to make sure this won't trip those of us who are less
>     proficient in Git (which, judging by the current traffic on
>     emacs-devel, seems like a rule rather than exception) and cause
>     corruption of the upstream repository.
>
> There are also other minor issues we need to figure out: licenses, the
> underlying infrastructure (e.g., AFIU, Phabricator requires PHP), etc.

Right, I'll have to investigate that. Licenses shouldn't be a problem (I 
only looked for Free Software packages), and as for infrastructure, 
someone should point out any particular problems that can turn out to be 
insurmountable. I can't think of any.

> Personally, I think arranging the development around this kind of
> process will not work without some critical mass of patch reviewers
> who are able to endure the current constant high volume of changes,
> let alone if we want to increase that volume.

Right, but that's only if/when we decide to push all contributions 
though this system. We could start light, and continue allowing 
committing directly to the repository, but move the 
patches-to-be-discussed (which will take up reviewers' time anyway) to 
the new system.

On the other hand, if we want to use this to guarantee quality of commit 
messages, a review process in some form will be required anyway, but 
since a lot of focus would be on the form rather than substance, not 
every reviewer would have to be intimately familiar with the area of the 
code they're reviewing.

Re: Patch queue management systems

[Eli Zaretskii]

> Date: Tue, 09 Dec 2014 15:03:00 +0200
> From: Dmitry Gutov <dgutov@yandex.ru>
> CC: monnier@iro.umontreal.ca, emacs-devel@gnu.org, eggert@cs.ucla.edu
> 
> On 12/06/2014 12:08 PM, Eli Zaretskii wrote:
> 
> > I think by far the 2 main issues we will have with these tools are:
> >
> >   . The tools don't have Emacs integration built into them, nor offer
> >     Emacs VC sub-modes.  (Gerrit has a magit plugin.)  I'm guessing we
> >     would like to arrange for such an integration in VC, because people
> >     who review patches are accustomed to using Emacs and have elaborate
> >     customizations for this that they would lose if the review is only
> >     done via a Web browser (even if that browser is eww).
> 
> Looking at magit-gerrit, it's not that big. If you're okay with that 
> level of integration, it shouldn't take too much work, as long as the 
> review system comes with a command-line tool or an API (which, of 
> course, will be a requirement).

Magit is not part of Emacs.  I think we would like this to be
integrated into VC.

> > Personally, I think arranging the development around this kind of
> > process will not work without some critical mass of patch reviewers
> > who are able to endure the current constant high volume of changes,
> > let alone if we want to increase that volume.
> 
> Right, but that's only if/when we decide to push all contributions 
> though this system. We could start light, and continue allowing 
> committing directly to the repository, but move the 
> patches-to-be-discussed (which will take up reviewers' time anyway) to 
> the new system.

I'm not sure such a split process makes sense.  It will complicate
procedures and most probably cause some patches fall through the
cracks.

> On the other hand, if we want to use this to guarantee quality of commit 
> messages, a review process in some form will be required anyway, but 
> since a lot of focus would be on the form rather than substance, not 
> every reviewer would have to be intimately familiar with the area of the 
> code they're reviewing.

I think comments about the form are usually the easiest part of the
review, and some of that can be automated.  Also, some form-related
issues are actually derived from familiarity with Emacs design and
implementation.  So yes, we could have "junior reviewers" who'd only
deal with form issues, but that would off-load only a minor portion of
the work required to do this.

Re: Patch queue management systems

[Dmitry Gutov]

On 12/09/2014 07:09 PM, Eli Zaretskii wrote:

> Magit is not part of Emacs.  I think we would like this to be
> integrated into VC.

Right. I meant reimplementing magit-gerrit won't take too much work.

>> Right, but that's only if/when we decide to push all contributions
>> though this system. We could start light, and continue allowing
>> committing directly to the repository, but move the
>> patches-to-be-discussed (which will take up reviewers' time anyway) to
>> the new system.
>
> I'm not sure such a split process makes sense.  It will complicate
> procedures and most probably cause some patches fall through the
> cracks.

Why? It's not much different from what happens now.

Consider this: if a patch is posted to the bug tracker or emacs-devel 
now, it would go though the new system when it's set up.

Patches that are written by users with commit access and installed 
directly will continue to proceed this way.

> I think comments about the form are usually the easiest part of the
> review, and some of that can be automated.  Also, some form-related
> issues are actually derived from familiarity with Emacs design and
> implementation.  So yes, we could have "junior reviewers" who'd only
> deal with form issues, but that would off-load only a minor portion of
> the work required to do this.

I don't know about that. If the code is written by a senior developer in 
the area where they're competent, most of the time only superficial 
review is required (unless the author explicitly indicates otherwise). 
Then a junior could be enough, too.

Re: Patch queue management systems

[Eli Zaretskii]

> Date: Tue, 09 Dec 2014 19:17:26 +0200
> From: Dmitry Gutov <dgutov@yandex.ru>
> CC: monnier@iro.umontreal.ca, emacs-devel@gnu.org, eggert@cs.ucla.edu
> 
> Patches that are written by users with commit access and installed 
> directly will continue to proceed this way.

I thought we wanted to make the review process mandatory for
everybody.

If we don't, we will have to come up with strict criteria for granting
commit access, because commit access is global and nowadays given to
people who only work on a small number of specific areas.

Re: Patch queue management systems

[Dmitry Gutov]

On 12/09/2014 07:36 PM, Eli Zaretskii wrote:

> I thought we wanted to make the review process mandatory for
> everybody.

A code review system can facilitate that workflow, but it doesn't 
require it.

> If we don't, we will have to come up with strict criteria for granting
> commit access, because commit access is global and nowadays given to
> people who only work on a small number of specific areas.

Yes, probably.

Either way, I'm ambivalent on the question of removing ChangeLogs, but 
doing that would allow to simplify automatic merging of patches.

Re: Patch queue management systems

[Ted Zlatanov]

On Tue, 09 Dec 2014 19:36:45 +0200 Eli Zaretskii <eliz@gnu.org> wrote: 

EZ> If we don't, we will have to come up with strict criteria for granting
EZ> commit access, because commit access is global and nowadays given to
EZ> people who only work on a small number of specific areas.

It could be more fine-grained, only protecting certain branches (master,
emacs-*, etc.).  Then people can push to "username/feature" branches and
maintainers can merge into a protected branch.

Ted

Re: Patch queue management systems

[Steinar Bang]

>>>>> Ted Zlatanov <tzz@lifelogs.com>:

> It could be more fine-grained, only protecting certain branches
> (master, emacs-*, etc.).  Then people can push to "username/feature"
> branches and maintainers can merge into a protected branch.

"username/feature" sounds like a good naming scheme and one that could
cut down on the inevitable clutter (implicit blame).

(and I still think having a separate feature branch git repository is a
good idea.  If that repository resides on the same server and is created
with hardlinks to the real repo, it should also be cheap
disk-usage-wise, at least initially....)

Re: Patch queue management systems

[Ted Zlatanov]

On Thu, 11 Dec 2014 09:59:28 +0100 Steinar Bang <sb@dod.no> wrote: 

>>>>>> Ted Zlatanov <tzz@lifelogs.com>:
>> It could be more fine-grained, only protecting certain branches
>> (master, emacs-*, etc.).  Then people can push to "username/feature"
>> branches and maintainers can merge into a protected branch.

SB> "username/feature" sounds like a good naming scheme and one that could
SB> cut down on the inevitable clutter (implicit blame).

SB> (and I still think having a separate feature branch git repository is a
SB> good idea.

Yeah, it would make allowing non-fast-forward pushes easier.

SB> If that repository resides on the same server and is created with
SB> hardlinks to the real repo, it should also be cheap disk-usage-wise,
SB> at least initially....)

Eh.

Ted

Re: Patch queue management systems

[Steinar Bang]

>>>>> Ted Zlatanov <tzz@lifelogs.com>:

SB> If that repository resides on the same server and is created with
SB> hardlinks to the real repo, it should also be cheap disk-usage-wise,
SB> at least initially....)

> Eh.

 cd /wherever/the/emacs/repo/lives
 git clone --mirror ./emacs.git ./emacs-feature-branches.git

If the git used is reasonably new, then all of the known-by-hash objects
in the new repository will be hardlinks to the original repo (which is
safe since these will never change).

So the new repository will initially use very little space on disk (all
history from the time of cloning will use the same inodes as the
original repository).

But as times goes and the emacs-feature-branches repo is filled by
clutter, the space usage of the feature branches repository will
increase.

Of course an easy way of doing housecleaning here, would be:
 cd /wherever/the/emacs/repo/lives
 rm -rf emacs-feature-branches.git
 git clone --mirror ./emacs.git ./emacs-feature-branches.git
and then let people repush their feature branches.

Re: Patch queue management systems

[Ted Zlatanov]

On Thu, 11 Dec 2014 13:38:15 +0100 Steinar Bang <sb@dod.no> wrote: 

>>>>>> Ted Zlatanov <tzz@lifelogs.com>:
SB> If that repository resides on the same server and is created with
SB> hardlinks to the real repo, it should also be cheap disk-usage-wise,
SB> at least initially....)

>> Eh.

SB>  cd /wherever/the/emacs/repo/lives
SB>  git clone --mirror ./emacs.git ./emacs-feature-branches.git

SB> If the git used is reasonably new, then all of the known-by-hash objects
SB> in the new repository will be hardlinks to the original repo (which is
SB> safe since these will never change).

I know all that (and the ways to override it) :)

I was saying that saving disk space is really low on the list of
benefits. Allowing non-fast-forward pushes for certain branches is much
more valuable to the Emacs project.

Stefan, Glenn: any chance we can get a contributor repo set up that's
just like the Emacs repo but allows non-fast-forward pushes, and whose
commits go to emacs-diffs?  Even better would be if user X could only
push to branches "X/.*"

Thanks
Ted

Re: Patch queue management systems

[David Kastrup]

Ted Zlatanov <tzz@lifelogs.com> writes:

> On Thu, 11 Dec 2014 13:38:15 +0100 Steinar Bang <sb@dod.no> wrote: 
>
>>>>>>> Ted Zlatanov <tzz@lifelogs.com>:
> SB> If that repository resides on the same server and is created with
> SB> hardlinks to the real repo, it should also be cheap disk-usage-wise,
> SB> at least initially....)
>
>>> Eh.
>
> SB>  cd /wherever/the/emacs/repo/lives
> SB>  git clone --mirror ./emacs.git ./emacs-feature-branches.git
>
> SB> If the git used is reasonably new, then all of the known-by-hash objects
> SB> in the new repository will be hardlinks to the original repo (which is
> SB> safe since these will never change).
>
> I know all that (and the ways to override it) :)
>
> I was saying that saving disk space is really low on the list of
> benefits. Allowing non-fast-forward pushes for certain branches is much
> more valuable to the Emacs project.
>
> Stefan, Glenn: any chance we can get a contributor repo set up that's
> just like the Emacs repo but allows non-fast-forward pushes, and whose
> commits go to emacs-diffs?  Even better would be if user X could only
> push to branches "X/.*"

Doesn't need a separate repo: a server side commit hook refusing
non-fast-forward pushes (more exactly: updating the reference after the
commit object is already there) on any path not starting, say, with
dev/, should be easily possible.

-- 
David Kastrup

Re: Patch queue management systems

[Ted Zlatanov]

On Thu, 11 Dec 2014 15:39:51 +0100 David Kastrup <dak@gnu.org> wrote: 

DK> Ted Zlatanov <tzz@lifelogs.com> writes:

>> Stefan, Glenn: any chance we can get a contributor repo set up that's
>> just like the Emacs repo but allows non-fast-forward pushes, and whose
>> commits go to emacs-diffs?  Even better would be if user X could only
>> push to branches "X/.*"

DK> Doesn't need a separate repo: a server side commit hook refusing
DK> non-fast-forward pushes (more exactly: updating the reference after the
DK> commit object is already there) on any path not starting, say, with
DK> dev/, should be easily possible.

Works for me (I've only set it up globally, so I didn't know it was
selectively possible).  Can you or one of the other admins set it up?
It would be very helpful.  A "dev/" prefix is fine by me, and makes it
easy to automatically fetch the relevant upstreams, something like:

[remote "origin"]
	fetch = +refs/dev/*/head:refs/remotes/origin/dev/*

Thanks
Ted

Re: Patch queue management systems

[Andreas Schwab]

David Kastrup <dak@gnu.org> writes:

> Doesn't need a separate repo: a server side commit hook refusing
> non-fast-forward pushes (more exactly: updating the reference after the
> commit object is already there) on any path not starting, say, with
> dev/, should be easily possible.

Doesn't even need that as long as you can delete branches.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: Patch queue management systems

[Ted Zlatanov]

On Thu, 11 Dec 2014 15:55:35 +0100 Andreas Schwab <schwab@suse.de> wrote: 

AS> David Kastrup <dak@gnu.org> writes:
>> Doesn't need a separate repo: a server side commit hook refusing
>> non-fast-forward pushes (more exactly: updating the reference after the
>> commit object is already there) on any path not starting, say, with
>> dev/, should be easily possible.

AS> Doesn't even need that as long as you can delete branches.

Doesn't even need that as long as we have Github.

Ted

Re: Patch queue management systems

[David Kastrup]

Ted Zlatanov <tzz@lifelogs.com> writes:

> On Thu, 11 Dec 2014 15:55:35 +0100 Andreas Schwab <schwab@suse.de> wrote: 
>
> AS> David Kastrup <dak@gnu.org> writes:
>>> Doesn't need a separate repo: a server side commit hook refusing
>>> non-fast-forward pushes (more exactly: updating the reference after the
>>> commit object is already there) on any path not starting, say, with
>>> dev/, should be easily possible.
>
> AS> Doesn't even need that as long as you can delete branches.
>
> Doesn't even need that as long as we have Github.

We don't "have" GitHub.  It's owned by a company and is running
proprietary software to which we don't have access.

-- 
David Kastrup

Re: Patch queue management systems

[Ted Zlatanov]

On Thu, 11 Dec 2014 16:26:21 +0100 David Kastrup <dak@gnu.org> wrote: 

DK> Ted Zlatanov <tzz@lifelogs.com> writes:
>> On Thu, 11 Dec 2014 15:55:35 +0100 Andreas Schwab <schwab@suse.de> wrote: 
>> 
AS> David Kastrup <dak@gnu.org> writes:
>>>> Doesn't need a separate repo: a server side commit hook refusing
>>>> non-fast-forward pushes (more exactly: updating the reference after the
>>>> commit object is already there) on any path not starting, say, with
>>>> dev/, should be easily possible.
>> 
AS> Doesn't even need that as long as you can delete branches.
>> 
>> Doesn't even need that as long as we have Github.

DK> We don't "have" GitHub.  It's owned by a company and is running
DK> proprietary software to which we don't have access.

Sorry, I forgot to add sarcasm tags.

Ted

Re: Patch queue management systems

[David Kastrup]

Ted Zlatanov <tzz@lifelogs.com> writes:

> On Thu, 11 Dec 2014 16:26:21 +0100 David Kastrup <dak@gnu.org> wrote: 
>
> DK> Ted Zlatanov <tzz@lifelogs.com> writes:
>>> On Thu, 11 Dec 2014 15:55:35 +0100 Andreas Schwab <schwab@suse.de> wrote: 
>>> 
> AS> David Kastrup <dak@gnu.org> writes:
>>>>> Doesn't need a separate repo: a server side commit hook refusing
>>>>> non-fast-forward pushes (more exactly: updating the reference after the
>>>>> commit object is already there) on any path not starting, say, with
>>>>> dev/, should be easily possible.
>>> 
> AS> Doesn't even need that as long as you can delete branches.
>>> 
>>> Doesn't even need that as long as we have Github.
>
> DK> We don't "have" GitHub.  It's owned by a company and is running
> DK> proprietary software to which we don't have access.
>
> Sorry, I forgot to add sarcasm tags.

It isn't sarcasm if an overwhelming majority of readers would not know
the difference.  This happens to be a publicly archived mailing list.

-- 
David Kastrup

Re: Patch queue management systems

[David Kastrup]

Andreas Schwab <schwab@suse.de> writes:

> David Kastrup <dak@gnu.org> writes:
>
>> Doesn't need a separate repo: a server side commit hook refusing
>> non-fast-forward pushes (more exactly: updating the reference after the
>> commit object is already there) on any path not starting, say, with
>> dev/, should be easily possible.
>
> Doesn't even need that as long as you can delete branches.

Yes and no.  Deleting and recreating a branch removes its reflog.  If
something goes wrong at the server side, it might be nice to have
around.  For basically ephemeral branches, this might be fine.  For
branches like master, one might want to disable branch deletion
altogether except for possibly some privileged users.  For, say,
/dev/username/* one might want to restrict branch
deletion/non-fastforward to the given username.

A commit hook can do such things IIRC.

-- 
David Kastrup

Re: Patch queue management systems

[Achim Gratz]

Steinar Bang writes:
> "username/feature" sounds like a good naming scheme and one that could
> cut down on the inevitable clutter (implicit blame).

I'd rather suggest features/<...>.  Anyone not wanting to look at
feature branches would simply not get those refs and everyone else can
either import single feature branches or all of them with an additional
line in the config file.

> (and I still think having a separate feature branch git repository is a
> good idea.  If that repository resides on the same server and is created
> with hardlinks to the real repo, it should also be cheap
> disk-usage-wise, at least initially....)

By using a branch prefix you can keep it in the same repository.
There's a multitude of ways to have a feature repository share the
objects with the main repository if you'd rather want that.


Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Wavetables for the Terratec KOMPLEXER:
http://Synth.Stromeko.net/Downloads.html#KomplexerWaves