Re: Flaw in our process for dealing with trivial changes

2019-12-13 Thread Richard Levitte
https://github.com/openssl/tools/pull/49 Quite an exercise, I think my python-fu has increased significantly. I have *no* idea how to debug CGI stuff in a sensible way, suggestions welcome! Cheers, Richard On Fri, 13 Dec 2019 12:08:42 +0100, Richard Levitte wrote: > > clacheck modification

Re: Flaw in our process for dealing with trivial changes

2019-12-13 Thread Richard Levitte
clacheck modification coming up! Cheers, Richard On Fri, 13 Dec 2019 04:48:38 +0100, Dr Paul Dale wrote: > > > A better example of this problem: #10607. Both Paul and I approved it > yesterday and I merged it > today without noticing until too late that it was tagged “CLA: trivial” :( > I’ve

Re: AW: Flaw in our process for dealing with trivial changes

2019-12-13 Thread Richard Levitte
On Fri, 13 Dec 2019 08:58:27 +0100, Dr. Matthias St. Pierre wrote: > > > > On Thu, 12 Dec 2019 22:31:10 +0100, > > Dr Paul Dale wrote: > > > > > > A red blocker along the lines of: "Triviality Unconfirmed". One of > > > the reviewers needs to remove this before the PR can be merged. > > > > > >

AW: Flaw in our process for dealing with trivial changes

2019-12-12 Thread Dr. Matthias St. Pierre
> On Thu, 12 Dec 2019 22:31:10 +0100, > Dr Paul Dale wrote: > > > > A red blocker along the lines of: "Triviality Unconfirmed". One of > > the reviewers needs to remove this before the PR can be merged. > > > > It's in our face, it prevent accidental merges and its low overhead. > > I still

Re: Flaw in our process for dealing with trivial changes

2019-12-12 Thread Dr Paul Dale
A better example of this problem: #10607. Both Paul and I approved it yesterday and I merged it today without noticing until too late that it was tagged “CLA: trivial” :( I’ve not reverted it at this point but will if necessary. Let’s get the label in. Pauli -- Dr Paul Dale | Distinguished

Re: Flaw in our process for dealing with trivial changes

2019-12-12 Thread Richard Levitte
On Thu, 12 Dec 2019 22:31:10 +0100, Dr Paul Dale wrote: > > A red blocker along the lines of: “Triviality Unconfirmed”. One of > the reviewers needs to remove this before the PR can be merged. > > It’s in our face, it prevent accidental merges and its low overhead. I still think simply adding

Re: AW: Flaw in our process for dealing with trivial changes

2019-12-12 Thread Richard Levitte
On Thu, 12 Dec 2019 12:15:30 +0100, Dr. Matthias St. Pierre wrote: > > 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. Correct re the clacheck hook (that's the

Re: Flaw in our process for dealing with trivial changes

2019-12-12 Thread 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

Re: Flaw in our process for dealing with trivial changes

2019-12-12 Thread Matt Caswell
On 12/12/2019 21:31, Dr Paul Dale wrote: > A red blocker along the lines of: “Triviality Unconfirmed”. One of the > reviewers needs to remove this before the PR can be merged. > > It’s in our face, it prevent accidental merges and its low overhead. Sounds workable. Matt > > > Pauli > -- 

Re: Flaw in our process for dealing with trivial changes

2019-12-12 Thread Dmitry Belyavsky
Great idea. пт, 13 дек. 2019 г., 0:31 Dr Paul Dale : > A red blocker along the lines of: “Triviality Unconfirmed”. One of the > reviewers needs to remove this before the PR can be merged. > > It’s in our face, it prevent accidental merges and its low overhead. > > > Pauli > -- > Dr Paul Dale |

Re: Flaw in our process for dealing with trivial changes

2019-12-12 Thread Dr Paul Dale
A red blocker along the lines of: “Triviality Unconfirmed”. One of the reviewers needs to remove this before the PR can be merged. It’s in our face, it prevent accidental merges and its low overhead. Pauli -- Dr Paul Dale | Distinguished Architect | Cryptographic Foundations Phone +61 7 3031

Re: Flaw in our process for dealing with trivial changes

2019-12-12 Thread Dr Paul Dale
Before we start over engineering a solution, how about we try just having an automatic visual indicator for trivial PRs. Pauli -- Dr Paul Dale | Distinguished Architect | Cryptographic Foundations Phone +61 7 3031 7217 Oracle Australia > On 13 Dec 2019, at 3:24 am, Kurt Roeckx wrote: >

Re: Flaw in our process for dealing with trivial changes

2019-12-12 Thread Dmitry Belyavsky
Dear Matt, On Thu, Dec 12, 2019 at 1:25 PM Matt Caswell wrote: > > On 12/12/2019 09:29, Dmitry Belyavsky wrote: > > - the contributor agreed to sign the CLA and > > - there was a mark that CLA is signed and > > - all the necessary approves were present > > I decided that there is no problem to

Re: Flaw in our process for dealing with trivial changes

2019-12-12 Thread Matt Caswell
On 12/12/2019 12:06, 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

Re: Flaw in our process for dealing with trivial changes

2019-12-12 Thread Dr. Matthias St. Pierre
> > 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

Re: AW: Flaw in our process for dealing with trivial changes

2019-12-12 Thread Matt Caswell
On 12/12/2019 11:15, Dr. Matthias St. Pierre wrote: > >> 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

AW: Flaw in our process for dealing with trivial changes

2019-12-12 Thread Dr. Matthias St. Pierre
> 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. > > > >

Re: Flaw in our process for dealing with trivial changes

2019-12-12 Thread Matt Caswell
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

Re: Flaw in our process for dealing with trivial changes

2019-12-12 Thread Paul Yang
It seems we have the same thoughts... Regards, Paul Yang > On Dec 12, 2019, at 5:36 PM, Dr Paul Dale wrote: > > I agree that there is a possible flaw in the workflow. What’s saved us so > far is that new contributors don’t generally include the "CLA: trivial" line > or put it in the GitHub

Re: Flaw in our process for dealing with trivial changes

2019-12-12 Thread Paul Yang
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. Regards, Paul Yang > On Dec 12, 2019, at 5:29 PM, Dmitry

Re: Flaw in our process for dealing with trivial changes

2019-12-12 Thread Dr Paul Dale
I agree that there is a possible flaw in the workflow. What’s saved us so far is that new contributors don’t generally include the "CLA: trivial" line or put it in the GitHub text. Could we have a “trivial” tag that is added whenever the "CLA: trivial" line is present? Better would be to add

Re: Flaw in our process for dealing with trivial changes

2019-12-12 Thread Dmitry Belyavsky
Dear Matt, As - the contributor agreed to sign the CLA and - there was a mark that CLA is signed and - all the necessary approves were present I decided that there is no problem to merge. BTW, I am not sure the PR was trivial enough. Anyway, the responsibility was mine, not the git one :) On

Flaw in our process for dealing with trivial changes

2019-12-12 Thread Matt Caswell
I notice that PR 10594 (Add support for otherName:NAIRealm in output) got merged yesterday: https://github.com/openssl/openssl/pull/10594 The commit description contained "CLA: trivial" and so the "hold: cla required" label was not automatically applied to the PR. But the discussion in the PR