Dear Matt, On Thu, Dec 12, 2019 at 1:25 PM Matt Caswell <m...@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 <beld...@gmail.com > >> <mailto:beld...@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 <m...@openssl.org > >> <mailto:m...@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