Flaw in our process for dealing with trivial changes
beldmit at gmail.com
Thu Dec 12 13:08:39 UTC 2019
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.
> > 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
> >> got merged yesterday:
> >> https://github.com/openssl/openssl/pull/10594
> >> The commit description contained "CLA: trivial" and so the "hold:
> >> 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
> >> 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...
More information about the openssl-project