Flaw in our process for dealing with trivial changes

[Richard Levitte]

On Thu, 12 Dec 2019 13:06:33 +0100,
Dr. Matthias St. Pierre wrote:
> 
> > > The server-side commit hook ensures that
> > >
> > > - the "CLA: trivial" annotation is present in *all* commits of the PR if and only if the [cla: trivial]
> > >   label is set.
> > > - the [cla: ok] label is set if and only if the CLA is on file
> > > - the pull request is accepted only if the [cla: ok] or [cla: trivial] label is set
> > 
> > 
> > One issue with this bit is that it requires the git hooks to connect to
> > github to check the labels. AFAIK we don't do that at present. Does it
> > add too much complexity to the hooks?
> 
> Actually, the consistency checks could be done entirely by the
> addrev script, which already uses the GitHub API.

Incidently, I'm looking at a rewrite of addrev to make it a
self-contained script that doesn't need the assistance of gitaddrev,
and that also computes everything in the preparation sweep, so the
filter part just needs to do the editing (i.e. should work *much*
faster).

> git commit hook
> =============
> 
> Accept pull request if and only if the CLA is on file or *all*
> commits have the "CLA: trivial" annotation.
> 
> (The git commit hook would need only a minimal change, if it does
> not check *all* commits yet.)

git education: It's the post-receive or update hooks, not the commit
hook.  There are commit hooks, and they are all client side, so
talking about a commit hook in this context is *really* confusing,
especially for those who aren't qutie aware.
Ref: https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks

That being said, the update hook does what you say, on a per commit
basis.  In other words, for each commit, it will do the appropriate
checks.  This is the leading comment of that script:

  # For any revision between oldrev and newrev, print VREF/UNREVIEWED/{sha}
  # if it hasn't been reviewed enough.
  #
  # The rules for being reviewed enough are:
  #
  # - UNLESS there's a 'CLA: Trivial' line:
  #   - at least one of the author and the reviewers MUST have a CLA and MUST
  #     be member of the 'omc' group.
  #   - at least two of the author and the reviewers MUST have a CLA and MUST
  #     be member of the 'commit' group.
  # - IF there's a 'CLA: Trivial' line:
  #   - at least one of the reviewers MUST have a CLA and MUST be member of the
  #     'omc' group.
  #   - at least two of the reviewers MUST have a CLA and MUST be member of the
  #     'commit' group.

(gitolite works by having a denial on any output VREF/UNREVIEWED/ line)

Cheers,
Richard

-- 
Richard Levitte         levitte at openssl.org
OpenSSL Project         http://www.openssl.org/~levitte/