Differences between revisions 1 and 12 (spanning 11 versions)
⇤ ← Revision 1 as of 2016-05-11 22:39:14 →
Size: 3692
Editor: mpm
Comment:
← Revision 12 as of 2021-07-30 03:00:01 → ⇥
Size: 4270
Editor: MattHarbison
Comment: minor config fixes
Deletions are marked like this. Additions are marked like this.
Line 2: Line 2:
Line 4: Line 3:
Line 12: Line 10:
Historically, [[mpm]] was the only developer with push access to the canonical Mercurial repo and reviewed every changeset. This process allows reviewers accept changes by recording multiple approvals. Every change accepted through this process must have at least two approvals distinct from its author.
Line 13: Line 12:
Historically, [[mpm]] has been the only developer with push access to the canonical Mercurial repo and reviewed every changeset.
This experimental process will allow reviewers to bypass mpm by recording multiple approvals. Every change accepted through this
process must have at least two approvals distinct from its author.		 The first approval is implicit in the push to the hg-committed repo, which can only be done by developers with push access. A pushlog records the author and pusher of each change.
Line 17: Line 14:
The first approval is implicit in the push to the hg-committed repo, which can only be done by developers with push access. A pushlog records the author and pusher of each change. The second approval is done by recording an entry in a ~/.accepted file. This can be done by developers with accept access, which is a subset of those with push access.
Line 19: Line 16:
The second approval is done by recording an entry in a ~/.accepted file. Similarly, this can be done only by users with push access (and thus a shell account on the server).

Lastly, a "land" cronjob consults the set of approvals and pulls all the commits that satisfy the invariants. In addition to the two reviewer rule, a commit can only "land" if its parent is already public or is itself landable.
It also keeps a log of the reviewers for each commit.		 Lastly, a "land" cronjob consults the set of approvals and pulls all the commits that satisfy the invariants. In addition to the two reviewer rule, a commit can only "land" if its parent is already public or is itself landable. It also keeps a log of the reviewers for each commit.
Line 25: Line 19:

The scripts for the accept process currently live in ~mpm/accept.
This currently just adds a full node id to your ~/.accepted file.
Eventually, it will also constrain the file to a reasonable length.
Line 33: Line 23:
accept = !ssh mercurial ~mpm/accept/accept $(hg log -r $1 -T '{node}') [alias]
accept = !ssh mercurial-scm.org /opt/accept/accept $(hg log -r $1 -T '{node}')
Line 35: Line 26:
Line 41: Line 31:
Line 44: Line 33:
(!) Speed this up with an ssh control master
Line 45: Line 36:
Line 55: Line 45:
pushlog = !ssh mercurial cat /srv/hgweb/repo/hg-committed/.hg/accept.log pushlog = !ssh mercurial-scm.org cat /srv/hgweb/repo/hg-committed/.hg/accept.log
Line 58: Line 48:
Secondly, there's a script ~mpm/accept/accepted that will consult the above log together with every reviewer's .accepted file and generate a list of commits with two or more accepts.
This is used as input to the "land" stage and can also be used to figure out which commits still need review.
Line 62: Line 50:

An experimental extension to support "external revsets" can be found at ~mpm/accept/extrevset.py.
It can let you wire a revset predicate to any shell command that returns a list of revisions.
This will let you consult the server for a list of changes that still need review right from your favorite "hg log" alias.		 An experimental {{{extdata()}}} revset can let you wire a revset predicate to any shell command that returns a list of revisions. This will let you consult the server for a list of changes that still need review right from your favorite "hg log" alias.
Line 68: Line 53:
[extensions]
extrevset=path/to/extrevset.py

[extrevset]
accepted = shell:ssh mercurial ~mpm/accept/accepted
canreview = shell:ssh mercurial ~mpm/accept/accepted | grep -v myusername		 [extdata]
accepted = shell:ssh mercurial-scm.org /opt/accept/accepted
canreview = shell:ssh mercurial-scm.org /opt/accept/reviewed | grep -v myusername
Line 76: Line 58:
review=log -G -r "draft() and canreview()" review = log -G -r "draft() and extdata(canreview)"
Line 79: Line 61:
== Setting up the template helper ==
An experimental {{{extdata()}}} template function allows shell-powered template keywords:

{{{
[extdata]
reviewers = shell:ssh mercurial-scm.org /opt/accept/reviewed
}}}
Add this to your favorite log template to get reviewer information:

{{{
$ hg log -r "draft()" -T "{rev} {extdata('reviewers')}\n"
}}}

== Rebasing and diffhashes ==

The accept process doesn't use obsolete markers because they're a little too generous. We don't want subsequent non-trivial edits to automatically get the sign-off of earlier accepters.

Instead, we calculate a "diffhash" of each diff that excludes line numbers, dates, and descriptions. It thus survives most of the trivial rebases and typo fixes in descriptions that we don't care about. This is used to carry forward accept markers from earlier commits with the same diffhash.

Keep in mind that a push of a rebased changeset '''counts as the second acceptance''' so be careful not to push anything you don't mean to.

/!\ Typo fixes in code comments DO invalidate the hash, so please get those cleaned up on the first pass.
Line 80: Line 85:
Line 85: Line 89:
 * template support for showing reviewers
Line 88: Line 91:
 * migration of the primary repo to mercurial-scm.org
Line 91: Line 93:
 * external source of truth for list of reviewers
Line 93: Line 94:
 * integration of scripts into the hginfra repository
== hgrc configuration ==

AugieFackler has published his [[https://bitbucket.org/snippets/durin42/74g9ME/hgrc-config-for-accept-process-on-core-hg|configuration]] for accept process work, in case others find it useful.
Line 96: Line 100:
## list categories here CategoryDeveloper

Accept Process

Note:

This page is primarily intended for developers of Mercurial.

An experimental process for reviewing contributions to Mercurial

Contents

  1. Overview
  2. Setting up the accept tool
  3. The pushlog and the "accepted" script
  4. Setting up the revset helper
  5. Setting up the template helper
  6. Rebasing and diffhashes
  7. Future work
  8. hgrc configuration

1. Overview

Historically, mpm was the only developer with push access to the canonical Mercurial repo and reviewed every changeset. This process allows reviewers accept changes by recording multiple approvals. Every change accepted through this process must have at least two approvals distinct from its author.

The first approval is implicit in the push to the hg-committed repo, which can only be done by developers with push access. A pushlog records the author and pusher of each change.

The second approval is done by recording an entry in a ~/.accepted file. This can be done by developers with accept access, which is a subset of those with push access.

Lastly, a "land" cronjob consults the set of approvals and pulls all the commits that satisfy the invariants. In addition to the two reviewer rule, a commit can only "land" if its parent is already public or is itself landable. It also keeps a log of the reviewers for each commit.

2. Setting up the accept tool

To smoothly integrate with a normal hg workflow, a shell alias is recommended: 

[alias]
accept = !ssh mercurial-scm.org /opt/accept/accept $(hg log -r $1 -T '{node}')

Then you can accept the current change thusly: 

$ hg accept .

/!\ Remember, the usual standards for review apply!

(!) Speed this up with an ssh control master

3. The pushlog and the "accepted" script

To see who pushed changeset, you can consult the pushlog. It is currently stored in two ways:

  • a permanent history of date, node, and pusher (/srv/hgweb/repo/hg-committed/.hg/push.log)
  • a list of draft changes with node, author, and pusher (/srv/hgweb/repo/hg-committed/.hg/accept.log)

This latter is pretty useful. You might want an alias to look at it: 

[alias]
pushlog = !ssh mercurial-scm.org cat /srv/hgweb/repo/hg-committed/.hg/accept.log

4. Setting up the revset helper

An experimental extdata() revset can let you wire a revset predicate to any shell command that returns a list of revisions. This will let you consult the server for a list of changes that still need review right from your favorite "hg log" alias. 

[extdata]
accepted = shell:ssh mercurial-scm.org /opt/accept/accepted
canreview = shell:ssh mercurial-scm.org /opt/accept/reviewed | grep -v myusername

[alias]
review = log -G -r "draft() and extdata(canreview)"

5. Setting up the template helper

An experimental extdata() template function allows shell-powered template keywords: 

[extdata]
reviewers = shell:ssh mercurial-scm.org /opt/accept/reviewed

Add this to your favorite log template to get reviewer information: 

$ hg log -r "draft()" -T "{rev} {extdata('reviewers')}\n"

6. Rebasing and diffhashes

The accept process doesn't use obsolete markers because they're a little too generous. We don't want subsequent non-trivial edits to automatically get the sign-off of earlier accepters.

Instead, we calculate a "diffhash" of each diff that excludes line numbers, dates, and descriptions. It thus survives most of the trivial rebases and typo fixes in descriptions that we don't care about. This is used to carry forward accept markers from earlier commits with the same diffhash.

Keep in mind that a push of a rebased changeset counts as the second acceptance so be careful not to push anything you don't mean to.

/!\ Typo fixes in code comments DO invalidate the hash, so please get those cleaned up on the first pass.

7. Future work

This sort of works today, but there's a lot more to do:

  • support accepting multiple commits
  • non-polled landing
  • hgweb support for showing reviewers
  • http-based queries to get to-review set
  • more sanity checks
  • a way to place temporary holds on commits
  • more knowledge about email aliases

8. hgrc configuration

AugieFackler has published his configuration for accept process work, in case others find it useful.

CategoryDeveloper

AcceptProcess (last edited 2021-07-30 03:00:01 by MattHarbison)