Flaw in our process for dealing with trivial changes

[Dmitry Belyavsky]

Dear Matt,

On Thu, Dec 12, 2019 at 1:25 PM Matt Caswell <matt at openssl.org> 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 merge.
>
> The only thing I could see in that PR was a message from the author
> saying that they would submit a CLA. There doesn't seem to be a message
> saying that the CLA had actually been processed.
>

Well, then it's my misinterpretation. I saw a author's words and green
checkbox related to CLA.
My fault, sorry.


> It's not sufficient for the author to *say* they have submitted a CLA.
> It must actually have been submitted, and accepted and be on file.
> Sometimes there are problems with submitted CLAs (e.g. missing fields,
> or a user sends an ICLA when they really also need a CCLA, etc).
> Normally the git hooks would not allow you to push a commit where the
> author does not have a CLA. However those checks are suppressed where
> "CLA: trivial" appears in the commit description. So we need to be extra
> careful in that case, i.e. perhaps we should insist that commits
> containing "CLA: trivial" have the line removed if we don't think the
> commit really is trivial. This ensures that the git hooks do their job
> and we can be absolutely sure that the author has a registered CLA.
>
> Matt
>
> >
> > Regards,
> >
> > Paul Yang
> >
> >> On Dec 12, 2019, at 5:29 PM, Dmitry Belyavsky <beldmit at gmail.com
> >> <mailto:beldmit at gmail.com>> wrote:
> >>
> >> 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 Thu, Dec 12, 2019 at 12:20 PM Matt Caswell <matt at openssl.org
> >> <mailto:matt at openssl.org>> wrote:
> >>
> >>     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 suggested a CLA should be submitted. But it got
> >>     merged anyway! Fortunately the CLA had already been processed -
> >>     just not
> >>     noted in the PR. So, in this case, it makes no difference.
> >>
> >>     I think this points to a possible flaw in our workflow for dealing
> >>     with
> >>     trivial changes. Because the "CLA: trivial" header suppresses the
> >>     "hold:
> >>     cla required" label and the git hooks don't complain when commits
> get
> >>     pushed with the "CLA: trivial" header and no CLA on file, it seems
> >>     possible to me that we could push commit all the way through the
> >>     process
> >>     without the reviewers even realising that the author is claiming
> >>     triviality on the commit.
> >>
> >>     Not sure what the solution to that is.
> >>
> >>     Matt
> >>
> >>
> >>
> >> --
> >> SY, Dmitry Belyavsky
> >
>


-- 
SY, Dmitry Belyavsky
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mta.openssl.org/pipermail/openssl-project/attachments/20191212/65684b7e/attachment.html>