Re: proposed change to committers policy

[Salz, Rich]

Nicely worded.  It doesn’t go as far as I’d like, but it’s a step.

Re: No two reviewers from same company

[Salz, Rich]

>   In that example the potential conflict of interest comes from the 
> individual's
employment with the third party organisation, not because they are fellows.
  
Do you disagree with my contention that the OMC represents the project, and not 
the fellows?

Regardless of where the conflict of interest comes from, the end result is the 
same, and I encourage the OMC to make the same policy for *its* employees that 
it does for other companies's employees.  Or perhaps this is a matter for the 
foundation to make for its employees.

Re: AW: [openssl] OpenSSL_1_1_1-stable update

[Matthias St. Pierre]

I forgot one word:

On 24.05.19 17:45, Matthias St. Pierre wrote:

(Otherwise, the missing approvers need to be from the Reviewed-by list
and additional approvals would be needed). 


need to be _removed_ from the Reviewed-by list

Re: AW: [openssl] OpenSSL_1_1_1-stable update

[Matthias St. Pierre]

On 24.05.19 16:54, Richard Levitte wrote:

On Fri, 24 May 2019 16:39:51 +0200,
Matt Caswell wrote:



On 24/05/2019 15:30, Richard Levitte wrote:

Not in practice.  We *do* ask on the PR in question if it should be
cherry-picked to 1.1.1 and seek approval for that action, but then it
hasn't at all been clear what should happen regarding Received-By
tags.

I have personally never touched them when cherry-picking, even in this
scenario.  I do not know what others do in that case...>


In principle I agree with Matt, his arguments are reasonable.
Maybe a compromise would be to allow the practice of asking
for an informal reconfirmation when cherry-pick tags were forgotten,
but only if one receives reconfirmation from _all_ original reviewers?
(Otherwise, the missing approvers need to be from the Reviewed-by list
and additional approvals would be needed).

Matthias

Re: AW: [openssl] OpenSSL_1_1_1-stable update

[Richard Levitte]

On Fri, 24 May 2019 16:39:51 +0200,
Matt Caswell wrote:
> 
> 
> 
> On 24/05/2019 15:30, Richard Levitte wrote:
> > 
> > Not in practice.  We *do* ask on the PR in question if it should be
> > cherry-picked to 1.1.1 and seek approval for that action, but then it
> > hasn't at all been clear what should happen regarding Received-By
> > tags.
> > 
> > I have personally never touched them when cherry-picking, even in this
> > scenario.  I do not know what others do in that case...>
> 
> In the vast majority of the cases the reviewers are the same.

Yes, because cherry-picking as an after-though rarely happens.  It's
mostly been along the lines of "hey, why was this bug-fix only applied
to master???"

> I wouldn't want other people putting my name in a reviewed-by tag
> where I have not approved it and I have not considered the
> implications of that change in that branch. What if it resulted in a
> critical CVE?

I haven't given possible CVEs much though, quite frankly.

But tell you what, I can certainly change my ways if this is what we
all think is the way to go.  Not a problem.  And I'm glad that we
talked about it, rather than staying with "I thought everyone else did
the same".

Cheers,
Richard

-- 
Richard Levitte levi...@openssl.org
OpenSSL Project http://www.openssl.org/~levitte/

Re: AW: [openssl] OpenSSL_1_1_1-stable update

[Richard Levitte]

On Fri, 24 May 2019 16:20:59 +0200,
Matt Caswell wrote:
> On 24/05/2019 15:10, Richard Levitte wrote:
> > If we go with the idea that an approval also involves approving what
> > branches it goes to, then what happens if someone realises after some
> > time that a set of commits (a PR) that was applied to master only
> > should really also be applied to 1.1.1?  Should the approval process
> > start over from scratch, i.e. all approvals that went to master should
> > be scratched and replaced with a new set of approvals (in principle)?
> 
> No. If the PR was approved for master and applied to master then no problem - 
> it
> stays in master. If it is later realised that it needs to be backported to 
> other
> branches then, yes, new approvals need to be sought for that change to *those
> branches*.
> 
> As far as I was aware we've always done this.

Not in practice.  We *do* ask on the PR in question if it should be
cherry-picked to 1.1.1 and seek approval for that action, but then it
hasn't at all been clear what should happen regarding Received-By
tags.

I have personally never touched them when cherry-picking, even in this
scenario.  I do not know what others do in that case...

-- 
Richard Levitte levi...@openssl.org
OpenSSL Project http://www.openssl.org/~levitte/

AW: [openssl] OpenSSL_1_1_1-stable update

[Dr. Matthias St. Pierre]

Matt and Richard, I think you are mixing up cherry-picking and nit-picking 
here. (Sorry for the pun ;-)

Matthias

> To be picky, may I assume that you meant a reviewed-by tag for you
> > should be *added*?  The commit itself (its contents) having been
> > reviewed by those already there, I cannot see a reason why they should
> > be removed.
> 
> You approved for master but did not approve for 1.1.1. This commit was merged 
> to
> 1.1.1 and so strictly speaking was not approved by you for that branch.
> Therefore you should have been removed, and I should have been added.
> 
> Matt

AW: proposed change to committers policy

[Dr. Matthias St. Pierre]

> > IMHO an important clarification needs to be made: The rule should not be 
> > about
> > _prohibiting_ double approvals. It should only be about _counting_ them.
> > I would not deprive people of the right to state their opinions.
> 
> Isn't that an implementation detail?

You're right, it should be. But current discussions and current practice show 
that Pauli
avoids to approve on GitHub when Shane is involved and vice versa. IMHO it 
should
be valid to approve (and also recorded in the commit message), even if the vote 
is not
counted. Because it documents that both have reviewed the code.

Matthias

Re: proposed change to committers policy

[Richard Levitte]

On Fri, 24 May 2019 11:21:18 +0200,
Matthias St. Pierre wrote:
> 
> >   In considering approvals, the combined approvals must come
> >   from individuals who work for separate organisations.
> >   This condition does not apply where the organisation is the
> >   OSF or OSS.
> 
> 
> IMHO an important clarification needs to be made: The rule should not be about
> _prohibiting_ double approvals. It should only be about _counting_ them.
> I would not deprive people of the right to state their opinions.

Isn't that an implementation detail?

-- 
Richard Levitte levi...@openssl.org
OpenSSL Project http://www.openssl.org/~levitte/

Re: proposed change to committers policy

[Matt Caswell]

On 24/05/2019 10:28, SHANE LONTIS wrote:
> It doesn’t stop us both reviewing a PR. That doesn’t mean we both need to 
> approve.

Right...but in Matthias's version if you raise a PR, and then Pauli approves it,
then you only then need to get a second committer approval. Otherwise you would
need to get an OMC approval.

That sounds ok to me.

Matt

> 
> 
>> On 24 May 2019, at 7:21 pm, Matthias St. Pierre > > wrote:
>>
>>
>>
>> >   In considering approvals, the combined approvals must come
>> >   from individuals who work for separate organisations.
>> >   This condition does not apply where the organisation is the
>> >   OSF or OSS.
>>
>>
>> IMHO an important clarification needs to be made: The rule should not be 
>> about
>> _prohibiting_ double approvals. It should only be about _counting_ them.
>> I would not deprive people of the right to state their opinions.
>>
>> Also, if Shane has already approved, then Pauli should still have the right
>> to approve, since his approval counts as OMC approval and Shane's not.
>>
>> How about using a formulation like the following?
>>
>>     In considering approvals, approvals from individuals being
>>         employed by the same third party company or organization should
>>         be counted as a single approval. If one of the individuals is
>>         an OMC member, the combined approval counts as an OMC approval.
>>     This condition does not apply where the organisation is the
>>     OSF or OSS.
>>
>>
>> An editorial comment: I think that the amendmend should be made inside the
>> unordered list, not following it.
>>
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openssl_web_blob_master_policies_committers.html-23L72-2DL78=DwIDaQ=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE=b1aL1L-m41VGkedIk-9Q7taAEKIshTBwq95Iah07uCk=DYHGYucybgbeMHQn9pS8SBhh_dvJkYrDgKu9gU2l9H4=jT3VTidhpkkMAwUuiu_JgqW9mBpD4PP-3Qc8D3KZDdU=
>>
>> Apart from that: Tim, could you (or someone else of the OMC) please raise a 
>> PR
>> for your proposal? That would make it easier to discuss the details.
>>
>> Matthias
>>
>>
> 

Re: proposed change to committers policy

[SHANE LONTIS]

It doesn’t stop us both reviewing a PR. That doesn’t mean we both need to 
approve.


> On 24 May 2019, at 7:21 pm, Matthias St. Pierre 
>  wrote:
> 
> 
> 
> >   In considering approvals, the combined approvals must come
> >   from individuals who work for separate organisations.
> >   This condition does not apply where the organisation is the
> >   OSF or OSS.
> 
> 
> IMHO an important clarification needs to be made: The rule should not be about
> _prohibiting_ double approvals. It should only be about _counting_ them.
> I would not deprive people of the right to state their opinions.
> 
> Also, if Shane has already approved, then Pauli should still have the right
> to approve, since his approval counts as OMC approval and Shane's not.
> 
> How about using a formulation like the following?
> 
> In considering approvals, approvals from individuals being
> employed by the same third party company or organization should
> be counted as a single approval. If one of the individuals is
> an OMC member, the combined approval counts as an OMC approval.
> This condition does not apply where the organisation is the
> OSF or OSS.
> 
> 
> An editorial comment: I think that the amendmend should be made inside the
> unordered list, not following it.
> 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openssl_web_blob_master_policies_committers.html-23L72-2DL78=DwIDaQ=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE=b1aL1L-m41VGkedIk-9Q7taAEKIshTBwq95Iah07uCk=DYHGYucybgbeMHQn9pS8SBhh_dvJkYrDgKu9gU2l9H4=jT3VTidhpkkMAwUuiu_JgqW9mBpD4PP-3Qc8D3KZDdU=
> 
> Apart from that: Tim, could you (or someone else of the OMC) please raise a PR
> for your proposal? That would make it easier to discuss the details.
> 
> Matthias
> 
>