AW: Flaw in our process for dealing with trivial changes

Dr. Matthias St. Pierre Matthias.St.Pierre at ncp-e.com
Thu Dec 12 11:15:30 UTC 2019


> On 12/12/2019 09:43, Paul Yang wrote:
> > Would it be better if 'CLA: trivial’ is in the commit message but no CLA
> > on file, then a new label like ’warn: no CLA but trivial’ is added? This
> > can inform the committer who will merge the PR for the CLA condition of
> > the commits.
> >
> 
> Yes, I think that would be a really good idea. At least then its very
> visible what is happening.

Reusing Tim's words I'd like to argue that we should avoid rushing for a 
premature optimization.

- Just adding new labels does not solve anything, at least as long as those labels
  are not enforced automatically. (via GitHub bot & git hook)

- This is the first time in quite a while that a pull request with a questionable
  trivial CLA has slipped in and it is no problem to revert a commit if necessary.
  So I wouldn't consider it a significant problem. The best countermeasure IMHO
  is to adopt the habit of skimming over the last GitHub messages of the PR and
  to reread the final commit messages (after the "Reviewed-by:" annotations have
  been added) before pushing the final commit.


As for a possible semi-automated solution:

The problem is more fundamental: currently both the GitHub bot and the git commit hook
only watch out for the 'CLA: trivial' marker. But this marker is intended to be added
by the *contributor* himself, so it expresses only his opinion, not ours. Only if all three,
the contributor and the two reviewers agree, then the pull request can be considered
trivial.

One possible solution to this problem could be the following procedure: 

Add three mutually exclusive [cla: *] labels:

  [cla: ok]                 (green)
  [cla: trivial]           (green)
  [cla: missing]        (red)

The CLA bot  *always* sets the [cla: ok] label if it finds a  CLA on file. Otherwise, it sets the
[cla: missing] label, unless the [cla: trivial] label is already set.

The [cla: trivial] label can only be set manually by a committer, and only after the consent
between contributor and both reviewers has been reached.

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


Matthias



More information about the openssl-project mailing list