the future of commit bits

--00d2d6ed898f474eb3eaa0405f3fb46d
Content-Type: text/plain

tl;dr:  We are going to slightly simplify the permissions on the perl5.git repository.  On top of that, we have a simple rule for who needs to agree that a merge commit should get merged.

What follows is broad strokes with some details yet to be refined, and I figure we'll refine them over time.  But this is the plan, and you should let us know what needs work.  I'll look at updating the pod in the near future.

Questions people sometimes ask:
 * how do I get a commit bit?
 * how do I get a PR approved?
 * can I merge this PR that looks good to me?
The pod about the repo doesn't really answer these questions.  The permissions on the repo are confusing to start, because there are a bunch of groups with very little explanation.  Here is our plan:

Write access to the repo is either *Committer* or *Releaser* access.  Both have write access to the repo.  Releasers have access for the limited scope of making one or more releases and doing only the required work to do that.  Committers have general-purpose write access, and use it to apply approved changes.  Committers *also* do the approving.

When can a merge request be merged?
 * If it's doc or test changes, one committer (other than the author) must approve the MR.
 * If it's a bugfix or changes to features, two committers (other than the author) must approve the MR.
 * When objections are raised, they should be discussed and addressed.  "Addressed" doesn't mean "everybody agrees," but that the rationale for making the change in the face of the objection is clearly stated.
 * Non-trivial MRs should wait to be merged for a minimum period of time so that discussion can occur.  How long?  It's proportional to the impact of the change.  A few days is a good start.  (A trivial MR is something like "fix a typo" or "rebuild perldoc".)
 * Subject matter expert committers can issue a veto.  If Karl says "this change to how we match Unicode properties will have serious negative impacts," he gets to block the merge at least until the situation is better understood.
 * When things seem stuck, contact Neil, Rik, or Sawyer to get things unstuck.
Who is in what teams?
 * Rik will be consolidating the too-many teams.  Current "full committers" will not be losing access.  Some releasers who don't do releases any more may.  Let Rik know if you seem to lose access and don't think you should.  It is probably a mistake.
 * After that, people are added to or removed from the permission groups by the steering council, who are happy to receive suggestions for new additions.
 * There may be some future removal of long-unused commit access, but nothing is planned right now.
I plan to start updating GitHub soon.  No matter what, our current permissions kind of a mess.

Feedback on this is hereby solicited!

-- 
rjbs
--00d2d6ed898f474eb3eaa0405f3fb46d
Content-Type: text/html
Content-Transfer-Encoding: quoted-printable

<!DOCTYPE html><html><head><title></title><style type=3D"text/css">p.Mso=
Normal,p.MsoNoSpacing{margin:0}</style></head><body><div>tl;dr:&nbsp; We=
 are going to slightly simplify the permissions on the perl5.git reposit=
ory.&nbsp; On top of that, we have a simple rule for who needs to agree =
that a merge commit should get merged.<br></div><div><br></div><div>What=
 follows is broad strokes with some details yet to be refined, and I fig=
ure we'll refine them over time.&nbsp; But this is the plan, and you sho=
uld let us know what needs work.&nbsp; I'll look at updating the pod in =
the near future.</div><div><br></div><div>Questions people sometimes ask=
:<br></div><ul><li>how do I get a commit bit?<br></li><li>how do I get a=
 PR approved?<br></li><li>can I merge this PR that looks good to me?<br>=
</li></ul><div>The pod about the repo doesn't really answer these questi=
ons.&nbsp; The permissions on the repo are confusing to start, because t=
here are a bunch of groups with very little explanation.&nbsp; Here is o=
ur plan:<br></div><div><br></div><div>Write access to the repo is either=
&nbsp;<b>Committer</b> or&nbsp;<b>Releaser</b> access.&nbsp; Both have w=
rite access to the repo.&nbsp; Releasers have access for the limited sco=
pe of making one or more releases and doing only the required work to do=
 that.&nbsp; Committers have general-purpose write access, and use it to=
 apply approved changes.&nbsp; Committers&nbsp;<i>also</i> do the approv=
ing.<br></div><div><br></div><div>When can a merge request be merged?<br=
></div><ul><li>If it's doc or test changes, one committer (other than th=
e author) must approve the MR.<br></li><li>If it's a bugfix or changes t=
o features, two committers (other than the author) must approve the MR.<=
br></li><li>When objections are raised, they should be discussed and add=
ressed.&nbsp; "Addressed" doesn't mean "everybody agrees," but that the =
rationale for making the change in the face of the objection is clearly =
stated.<br></li><li>Non-trivial MRs should wait to be merged for a minim=
um period of time so that discussion can occur.&nbsp; How long?&nbsp; It=
's proportional to the impact of the change.&nbsp; A few days is a good =
start.&nbsp; (A trivial MR is something like "fix a typo" or "rebuild pe=
rldoc".)<br></li><li>Subject matter expert committers can issue a veto.&=
nbsp; If Karl says "this change to how we match Unicode properties will =
have serious negative impacts," he gets to block the merge at least unti=
l the situation is better understood.<br></li><li>When things seem stuck=
, contact Neil, Rik, or Sawyer to get things unstuck.<br></li></ul><div>=
Who is in what teams?<br></div><ul><li>Rik will be consolidating the too=
-many teams.&nbsp; Current "full committers" will not be losing access.&=
nbsp; Some releasers who don't do releases any more may.&nbsp; Let Rik k=
now if you seem to lose access and don't think you should.&nbsp; It is p=
robably a mistake.<br></li><li>After that, people are added to or remove=
d from the permission groups by the steering council, who are happy to r=
eceive suggestions for new additions.<br></li><li>There may be some futu=
re removal of long-unused commit access, but nothing is planned right no=
w.</li></ul><div>I plan to start updating GitHub soon.&nbsp; No matter w=
hat, our current permissions kind of a mess.<br></div><div><br></div><di=
v>Feedback on this is hereby solicited!</div><div><br></div><div>--&nbsp=
;<br></div><div>rjbs</div></body></html>
--00d2d6ed898f474eb3eaa0405f3fb46d--
0
rjbs
1/12/2021 6:57:31 PM
perl.perl5.porters 48287 articles. 1 followers. Follow

13 Replies
240 Views

Similar Articles

[PageSpeed] 26

On Tue, Jan 12, 2021 at 01:57:31PM -0500, Ricardo Signes wrote:
> tl;dr:  We are going to slightly simplify the permissions on the perl5.git repository.  On top of that, we have a simple rule for who needs to agree that a merge commit should get merged.
> 
> What follows is broad strokes with some details yet to be refined, and I figure we'll refine them over time.  But this is the plan, and you should let us know what needs work.  I'll look at updating the pod in the near future.
> 
> Questions people sometimes ask:
>  * how do I get a commit bit?
>  * how do I get a PR approved?
>  * can I merge this PR that looks good to me?
> The pod about the repo doesn't really answer these questions.  The permissions on the repo are confusing to start, because there are a bunch of groups with very little explanation.  Here is our plan:
> 
> Write access to the repo is either *Committer* or *Releaser* access.  Both have write access to the repo.  Releasers have access for the limited scope of making one or more releases and doing only the required work to do that.  Committers have general-purpose write access, and use it to apply approved changes.  Committers *also* do the approving.
> 
> When can a merge request be merged?
>  * If it's doc or test changes, one committer (other than the author) must approve the MR.
>  * If it's a bugfix or changes to features, two committers (other than the author) must approve the MR.

I like the idea, but I don't think we have enough active committers for it to work.

Are committers permitted to commit directly or are they required to
make a MR? (implied, but not explicitly mentioned)

Tony
0
tony
1/12/2021 10:42:21 PM
On Wed, 13 Jan 2021 09:42:21 +1100
Tony Cook <tony@develop-help.com> wrote:

> I like the idea, but I don't think we have enough active committers
> for it to work.

I'd be happy to do more code-review kinds of things - feel free to
involve me more in such things. :)

-- 
Paul "LeoNerd" Evans

leonerd@leonerd.org.uk      |  https://metacpan.org/author/PEVANS
http://www.leonerd.org.uk/  |  https://www.tindie.com/stores/leonerd/
0
leonerd
1/12/2021 11:01:10 PM
--c014c43b425449df9c102b069799877e
Content-Type: text/plain

On Tue, Jan 12, 2021, at 5:42 PM, Tony Cook wrote:
> I like the idea, but I don't think we have enough active committers for it to work.

Putting aside what I or anybody else thinks:  what do you think about the realism if we include the author in the count, if they are a committer?
 * typo/trivial fix requires one committer, which may be the author
 * larger change requires two committers, one of which may be the author?

> Are committers permitted to commit directly or are they required to make a MR? (implied, but not explicitly mentioned)

I don't know that we made it very explicit in our discussion, but I believe that a committer would not commit directly, but would instead make an MR.  I realize this makes an impact on the pace of making small changes, and am interested in your feedback (and that of the rest of the crew).

-- 
rjbs
--c014c43b425449df9c102b069799877e
Content-Type: text/html
Content-Transfer-Encoding: quoted-printable

<!DOCTYPE html><html><head><title></title><style type=3D"text/css">p.Mso=
Normal,p.MsoNoSpacing{margin:0}</style></head><body><div>On Tue, Jan 12,=
 2021, at 5:42 PM, Tony Cook wrote:<br></div><blockquote type=3D"cite" i=
d=3D"qt" style=3D""><div>I like the idea, but I don't think we have enou=
gh active committers for it to work.<br></div></blockquote><div><br></di=
v><div>Putting aside what I or anybody else thinks:&nbsp; what do you th=
ink about the realism if we include the author in the count, if they are=
 a committer?<br></div><ul><li>typo/trivial fix requires one committer, =
which may be the author<br></li><li>larger change requires two committer=
s, one of which may be the author?</li></ul><div><br></div><blockquote t=
ype=3D"cite" id=3D"qt" style=3D""><div>Are committers permitted to commi=
t directly or are they required to&nbsp;make a MR? (implied, but not exp=
licitly mentioned)<br></div></blockquote><div><br></div><div>I don't kno=
w that we made it very explicit in our discussion, but I believe that a =
committer would not commit directly, but would instead make an MR.&nbsp;=
 I realize this makes an impact on the pace of making small changes, and=
 am interested in your feedback (and that of the rest of the crew).<br><=
/div><div><br></div><div>-- <br></div><div>rjbs<br></div></body></html>
--c014c43b425449df9c102b069799877e--
0
rjbs
1/12/2021 11:08:21 PM
--000000000000af853605b8bc67fd
Content-Type: text/plain; charset="UTF-8"

On Tue, Jan 12, 2021 at 3:16 PM Ricardo Signes <rjbs@manxome.org> wrote:

> I don't know that we made it very explicit in our discussion, but I
> believe that a committer would not commit directly, but would instead make
> an MR.  I realize this makes an impact on the pace of making small changes,
> and am interested in your feedback (and that of the rest of the crew).
>

Perhaps others might feel strongly the other way, but personally I would
not be inclined to create a pull request unless I intended to post the
changes for review/comment, and if the change is small enough (and in a
single commit) I wouldn't even make a separate branch for it, but just
commit directly on blead.

--000000000000af853605b8bc67fd
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><br></div><div class=3D"gmail_quote"><div=
 dir=3D"ltr" class=3D"gmail_attr">On Tue, Jan 12, 2021 at 3:16 PM Ricardo S=
ignes &lt;<a href=3D"mailto:rjbs@manxome.org">rjbs@manxome.org</a>&gt; wrot=
e:<br></div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0=
..8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div>I d=
on&#39;t know that we made it very explicit in our discussion, but I believ=
e that a committer would not commit directly, but would instead make an MR.=
=C2=A0 I realize this makes an impact on the pace of making small changes, =
and am interested in your feedback (and that of the rest of the crew).<br><=
/div></div></blockquote><div><br></div><div>Perhaps others might feel stron=
gly the other way, but personally I would not be inclined to create a pull =
request unless I intended to post the changes for review/comment, and if th=
e change is small enough (and in a single commit) I wouldn&#39;t even make =
a separate branch for it, but just commit directly on blead.</div><div><br>=
</div><div> <br></div></div></div>

--000000000000af853605b8bc67fd--
0
perl
1/12/2021 11:30:00 PM
On Tue, Jan 12, 2021 at 11:42 PM Tony Cook <tony@develop-help.com> wrote:
>
> On Tue, Jan 12, 2021 at 01:57:31PM -0500, Ricardo Signes wrote:
> > tl;dr:  We are going to slightly simplify the permissions on the perl5.=
git repository.  On top of that, we have a simple rule for who needs to agr=
ee that a merge commit should get merged.
> >
> > What follows is broad strokes with some details yet to be refined, and =
I figure we'll refine them over time.  But this is the plan, and you should=
 let us know what needs work.  I'll look at updating the pod in the near fu=
ture.
> >
> > Questions people sometimes ask:
> >  * how do I get a commit bit?
> >  * how do I get a PR approved?
> >  * can I merge this PR that looks good to me?
> > The pod about the repo doesn't really answer these questions.  The perm=
issions on the repo are confusing to start, because there are a bunch of gr=
oups with very little explanation.  Here is our plan:
> >
> > Write access to the repo is either *Committer* or *Releaser* access.  B=
oth have write access to the repo.  Releasers have access for the limited s=
cope of making one or more releases and doing only the required work to do =
that.  Committers have general-purpose write access, and use it to apply ap=
proved changes.  Committers *also* do the approving.
> >
> > When can a merge request be merged?
> >  * If it's doc or test changes, one committer (other than the author) m=
ust approve the MR.
> >  * If it's a bugfix or changes to features, two committers (other than =
the author) must approve the MR.
>
> I like the idea, but I don't think we have enough active committers for i=
t to work.
>
> Are committers permitted to commit directly or are they required to
> make a MR? (implied, but not explicitly mentioned)

This is a concern. I don't think this will work without commitment
from the committers.

That said, I do think more reviewing than currently happens is a good
idea, I'm in.

Leon
0
fawaka
1/12/2021 11:40:55 PM
--000000000000a615c505b8bd1e21
Content-Type: text/plain; charset="UTF-8"

On Tue, 12 Jan 2021, 23:16 Ricardo Signes, <rjbs@manxome.org> wrote:

> On Tue, Jan 12, 2021, at 5:42 PM, Tony Cook wrote:
>
> I like the idea, but I don't think we have enough active committers for it
> to work.
>
>
> Putting aside what I or anybody else thinks:  what do you think about the
> realism if we include the author in the count, if they are a committer?
>
>    - typo/trivial fix requires one committer, which may be the author
>    - larger change requires two committers, one of which may be the
>    author?
>
> That sounds good to me. It stops short of creating a needless burden for
trivial changes, but stills introduces code reviews for larger changes.

That's exactly the system my $workplace has used for as long as I can
remember and has always worked well there.



> Are committers permitted to commit directly or are they required to make a
> MR? (implied, but not explicitly mentioned)
>
>
> I don't know that we made it very explicit in our discussion, but I
> believe that a committer would not commit directly, but would instead make
> an MR.  I realize this makes an impact on the pace of making small changes,
> and am interested in your feedback (and that of the rest of the crew).
>

A committer would commit trivial changes directly with your revised plan
above, so sufficiently small changes would be unaffected.

It does require honesty and discipline on the part of the committers to not
abuse the system, but it's never been an issue at $work and I would
certainly trust people here to likewise exercise good judgement over
whether a change needs review or not.

--000000000000a615c505b8bd1e21
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"auto"><div><div class=3D"gmail_quote"><div dir=3D"ltr" class=3D=
"gmail_attr">On Tue, 12 Jan 2021, 23:16 Ricardo Signes, &lt;<a href=3D"mail=
to:rjbs@manxome.org">rjbs@manxome.org</a>&gt; wrote:<br></div><blockquote c=
lass=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px #ccc solid;=
padding-left:1ex"><u></u><div><div>On Tue, Jan 12, 2021, at 5:42 PM, Tony C=
ook wrote:<br></div><blockquote type=3D"cite" id=3D"m_-5747251642961803974q=
t"><div>I like the idea, but I don&#39;t think we have enough active commit=
ters for it to work.<br></div></blockquote><div><br></div><div>Putting asid=
e what I or anybody else thinks:=C2=A0 what do you think about the realism =
if we include the author in the count, if they are a committer?<br></div><u=
l><li>typo/trivial fix requires one committer, which may be the author<br><=
/li><li>larger change requires two committers, one of which may be the auth=
or?</li></ul></div></blockquote></div></div><div dir=3D"auto">That sounds g=
ood to me. It stops short of creating a needless burden for trivial changes=
, but stills introduces code reviews for larger changes.</div><div dir=3D"a=
uto"><br></div><div dir=3D"auto">That&#39;s exactly the system my $workplac=
e has used for as long as I can remember and has always worked well there.<=
/div><div dir=3D"auto"><br></div><div dir=3D"auto"><br></div><div dir=3D"au=
to"><div class=3D"gmail_quote"><blockquote class=3D"gmail_quote" style=3D"m=
argin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir=3D"a=
uto"><br></div><blockquote type=3D"cite" id=3D"m_-5747251642961803974qt"><d=
iv>Are committers permitted to commit directly or are they required to=C2=
=A0make a MR? (implied, but not explicitly mentioned)<br></div></blockquote=
><div dir=3D"auto"><br></div><div dir=3D"auto">I don&#39;t know that we mad=
e it very explicit in our discussion, but I believe that a committer would =
not commit directly, but would instead make an MR.=C2=A0 I realize this mak=
es an impact on the pace of making small changes, and am interested in your=
 feedback (and that of the rest of the crew).</div></blockquote></div></div=
><div dir=3D"auto"><br></div><div dir=3D"auto">A committer would commit tri=
vial changes directly with your revised plan above, so sufficiently small c=
hanges would be unaffected.<br></div><div dir=3D"auto"><br></div><div dir=
=3D"auto">It does require honesty and discipline on the part of the committ=
ers to not abuse the system, but it&#39;s never been an issue at $work and =
I would certainly trust people here to likewise exercise good judgement ove=
r whether a change needs review or not.</div><div dir=3D"auto"></div></div>

--000000000000a615c505b8bd1e21--
0
perl5
1/13/2021 12:21:18 AM
On Tue, Jan 12, 2021 at 06:08:21PM -0500, Ricardo Signes wrote:
> On Tue, Jan 12, 2021, at 5:42 PM, Tony Cook wrote:
> > I like the idea, but I don't think we have enough active committers for it to work.
> 
> Putting aside what I or anybody else thinks:  what do you think about the realism if we include the author in the count, if they are a committer?
>  * typo/trivial fix requires one committer, which may be the author
>  * larger change requires two committers, one of which may be the author?

I think it's more realistic.

> > Are committers permitted to commit directly or are they required to make a MR? (implied, but not explicitly mentioned)
> 
> I don't know that we made it very explicit in our discussion, but I believe that a committer would not commit directly, but would instead make an MR.  I realize this makes an impact on the pace of making small changes, and am interested in your feedback (and that of the rest of the crew).

I've tended to make a PR for non-trivial changes (a github PR now, a
patch attached to a ticket with RT, which led to other problems in RT
times)

I can see a need for direct commits in emergencies, specifically in
the case of a broken build.  These are typically trivial commits though.

I can see it being a problem for small but non-trivial changes
(wherever you happen to draw those lines.)



Tony
0
tony
1/13/2021 1:14:52 AM
On 1/12/21 6:14 PM, Tony Cook wrote:
> On Tue, Jan 12, 2021 at 06:08:21PM -0500, Ricardo Signes wrote:
>> On Tue, Jan 12, 2021, at 5:42 PM, Tony Cook wrote:
>>> I like the idea, but I don't think we have enough active committers for it to work.
>>
>> Putting aside what I or anybody else thinks:  what do you think about the realism if we include the author in the count, if they are a committer?
>>   * typo/trivial fix requires one committer, which may be the author
>>   * larger change requires two committers, one of which may be the author?
> 
> I think it's more realistic.

I agree that this would be the ideal world solution, and the revision is 
more realistic, but I doubt that it is still realistic enough.

A huge percentage of my commits these past few months have been 
Warnocked.  After a while I committed them anyway, and just merged the 
ones I was confident about directly.

I have eventually committed other people's work that I did not 
understand fully, when no one else did, because at that point in the 
development cycle, it is important to keep things moving, and to 
encourage people to continue on the project.  I tried to get private buy 
in from non-committers who have a good track record, in my opinion, 
before doing so.  But I was not always successful in getting them to 
engage.  A thumb's up on the PR encourages me.  I haven't regretted any 
of them.  (I did look, and no trojan horses jumped out at me.)

Before I had a commit bit, an important commit of mine languished for 6 
months because it was complicated and dealing in an area few if any 
people still on the project knew much about.  I could easily have packed 
up my marbles and gone elsewhere, and I was tempted to.  That commit led 
to fixing many of the qr//i bugs with Unicode, which the project didn't 
even know it had.  It would have been better to merge it early in the 
cycle even without understanding it, knowing there was time to fix the 
bugs or revert it before ship date.

I don't know what the answer is.  I do think project timing should be a 
consideration.
> 
>>> Are committers permitted to commit directly or are they required to make a MR? (implied, but not explicitly mentioned)
>>
>> I don't know that we made it very explicit in our discussion, but I believe that a committer would not commit directly, but would instead make an MR.  I realize this makes an impact on the pace of making small changes, and am interested in your feedback (and that of the rest of the crew).
> 
> I've tended to make a PR for non-trivial changes (a github PR now, a
> patch attached to a ticket with RT, which led to other problems in RT
> times)
> 
> I can see a need for direct commits in emergencies, specifically in
> the case of a broken build.  These are typically trivial commits though.
> 
> I can see it being a problem for small but non-trivial changes
> (wherever you happen to draw those lines.)
> 
> 
> 
> Tony
> 

0
public
1/13/2021 4:24:26 AM
On Tue, Jan 12, 2021 at 10:24 PM Karl Williamson
<public@khwilliamson.com> wrote:
>
> On 1/12/21 6:14 PM, Tony Cook wrote:
> > On Tue, Jan 12, 2021 at 06:08:21PM -0500, Ricardo Signes wrote:
> >> On Tue, Jan 12, 2021, at 5:42 PM, Tony Cook wrote:
> >>> I like the idea, but I don't think we have enough active committers for it to work.
> >>
> >> Putting aside what I or anybody else thinks:  what do you think about the realism if we include the author in the count, if they are a committer?
> >>   * typo/trivial fix requires one committer, which may be the author
> >>   * larger change requires two committers, one of which may be the author?
> >
> > I think it's more realistic.
>
> I agree that this would be the ideal world solution, and the revision is
> more realistic, but I doubt that it is still realistic enough.
>
> A huge percentage of my commits these past few months have been
> Warnocked.  After a while I committed them anyway, and just merged the
> ones I was confident about directly.
>
> I have eventually committed other people's work that I did not
> understand fully, when no one else did, because at that point in the
> development cycle, it is important to keep things moving, and to
> encourage people to continue on the project.  I tried to get private buy
> in from non-committers who have a good track record, in my opinion,
> before doing so.  But I was not always successful in getting them to
> engage.  A thumb's up on the PR encourages me.  I haven't regretted any
> of them.  (I did look, and no trojan horses jumped out at me.)
>
> Before I had a commit bit, an important commit of mine languished for 6
> months because it was complicated and dealing in an area few if any
> people still on the project knew much about.  I could easily have packed
> up my marbles and gone elsewhere, and I was tempted to.  That commit led
> to fixing many of the qr//i bugs with Unicode, which the project didn't
> even know it had.  It would have been better to merge it early in the
> cycle even without understanding it, knowing there was time to fix the
> bugs or revert it before ship date.
>
> I don't know what the answer is.  I do think project timing should be a
> consideration.

I don't know either, but I agree that there is the potential for
throwing sand in the gears of getting work done.  I know that code
review is considered "best practices" and it is certainly a nice thing
to have if you can afford it.  I'm sceptical whether we can afford it.
I've always wished for more feedback on my work in various settings
(not just Perl) and seldom found people who had the time to provide
it. Traditionally commit bits have been handed out when folks got
tired of reviewing patches outside of their area of expertise and the
patch submitter had a generally good reputation for work that didn't
cause trouble.

People have mentioned following a similar process at $work. We have
recently started a similar process where I work and the code reviewers
are swamped. One of them recently told me he's lucky to get two hours
a day to do his own work. In a business setting this can theoretically
be managed by designating more resources, but I don't see that
happening in a volunteer setting.

There is also the opportunity cost of adding another step that is
particularly painful for occasional contributors like me.  If I only
get a couple of hours every seventh Sunday to do Perl-related work,
waiting a few weeks for a code review would pretty much put that into
the why bother category.
0
craig
1/14/2021 4:43:37 AM
On Thu, 14 Jan 2021 at 04:43, Craig A. Berry <craig.a.berry@gmail.com> wrote:
>
> On Tue, Jan 12, 2021 at 10:24 PM Karl Williamson
> <public@khwilliamson.com> wrote:
> >
> > On 1/12/21 6:14 PM, Tony Cook wrote:
> > > On Tue, Jan 12, 2021 at 06:08:21PM -0500, Ricardo Signes wrote:
> > >> On Tue, Jan 12, 2021, at 5:42 PM, Tony Cook wrote:
> > >>> I like the idea, but I don't think we have enough active committers for it to work.
> > >>
> > >> Putting aside what I or anybody else thinks:  what do you think about the realism if we include the author in the count, if they are a committer?
> > >>   * typo/trivial fix requires one committer, which may be the author
> > >>   * larger change requires two committers, one of which may be the author?
> > >
> > > I think it's more realistic.
> >
> > I agree that this would be the ideal world solution, and the revision is
> > more realistic, but I doubt that it is still realistic enough.
> >
> > A huge percentage of my commits these past few months have been
> > Warnocked.  After a while I committed them anyway, and just merged the
> > ones I was confident about directly.
> >
> > I have eventually committed other people's work that I did not
> > understand fully, when no one else did, because at that point in the
> > development cycle, it is important to keep things moving, and to
> > encourage people to continue on the project.  I tried to get private buy
> > in from non-committers who have a good track record, in my opinion,
> > before doing so.  But I was not always successful in getting them to
> > engage.  A thumb's up on the PR encourages me.  I haven't regretted any
> > of them.  (I did look, and no trojan horses jumped out at me.)
> >
> > Before I had a commit bit, an important commit of mine languished for 6
> > months because it was complicated and dealing in an area few if any
> > people still on the project knew much about.  I could easily have packed
> > up my marbles and gone elsewhere, and I was tempted to.  That commit led
> > to fixing many of the qr//i bugs with Unicode, which the project didn't
> > even know it had.  It would have been better to merge it early in the
> > cycle even without understanding it, knowing there was time to fix the
> > bugs or revert it before ship date.
> >
> > I don't know what the answer is.  I do think project timing should be a
> > consideration.

That's a good point. I'm sure we have committed risky-ish things
earlier in dev cycles at $work, sometimes even deliberately leaving a
commit until the next dev cycle starts before committing it, where
there is more of an unknown quantity in the change. Sometimes it just
isn't feasible to review and test changes to the usual standards for
various reasons (e.g. hoary old code that rarely gets touched, written
by people who have long since left).

>
> I don't know either, but I agree that there is the potential for
> throwing sand in the gears of getting work done.  I know that code
> review is considered "best practices" and it is certainly a nice thing
> to have if you can afford it.  I'm sceptical whether we can afford it.
> I've always wished for more feedback on my work in various settings
> (not just Perl) and seldom found people who had the time to provide
> it. Traditionally commit bits have been handed out when folks got
> tired of reviewing patches outside of their area of expertise and the
> patch submitter had a generally good reputation for work that didn't
> cause trouble.
>
> People have mentioned following a similar process at $work. We have
> recently started a similar process where I work and the code reviewers
> are swamped. One of them recently told me he's lucky to get two hours
> a day to do his own work. In a business setting this can theoretically
> be managed by designating more resources, but I don't see that
> happening in a volunteer setting.
>
> There is also the opportunity cost of adding another step that is
> particularly painful for occasional contributors like me.  If I only
> get a couple of hours every seventh Sunday to do Perl-related work,
> waiting a few weeks for a code review would pretty much put that into
> the why bother category.

At $work it swings this way and that: Sometimes person A writes so
much code quickly that person B is swamped reviewing/testing it for a
week or more, but sooner or later person A will get stalled for a
while (either fixing problems found in review/testing, or working on
another task that isn't so easy) - and then person B gets on a roll
writing code.

I can see that being more of a problem here, though, because some
people (especially those being paid TPF grants, and maybe some others
in $workplaces that sanction them spending some time on perl) spend
many more hours on the project than other occasional contributors. The
occasional contributors surely won't want to spend much of their time
reviewing, but neither will the more active contributors want to have
to do all the reviewing.
0
perl5
1/14/2021 8:38:29 AM
--5a720527d7484a05904517687bc40a3c
Content-Type: text/plain

Thanks for the feedback!  I think there are couple key things that got called out:
 * a bunch of people want more review
 * there is skepticism that saying its required will make it happen
 * nobody wants to get stuck behind a wall of silence
Part of the impetus for this discussion was another point:
 * nobody wants to get stuck when they get a volunteer reviewer who foot-drags or nit-picks
I also appreciated this comment from Craig:
> Traditionally commit bits have been handed out when folks got tired of reviewing patches outside of their area of expertise and the patch submitter had a generally good reputation for work that didn't cause trouble.

Altogether, maybe a structure like this is better:
 * committers should commit their work after an appropriate review period spent as a pull request
 * trivial or obviously-correct (!) changes may be committed directly (i.e., the appropriate length is sometimes zero)
 * if objections are raised, they need to be addressed (meaning "clearly replied to")
 * if the objection is from a subject matter expert, you need to come to an agreement
 * if the above fails, ping neilb/rjbs/sawyer for adjudication, which I expect will be quite rare
I think this is very simple, very close to what we do now, and avoids "I was waiting a very long time" because one is free to say, "Well, I am applying this as I can't get feedback and believe it is correct.

The final things to mention are probably:
 * for "I think this is right but want to give a chance for comments", a couple days is probably fine
 * for "this is a significant change that I could really use feedback on," a week or more is probably best, and the PR should probably be flagged to the list as wanting attention
 * the later in the release cycle, the stricter we should be with ourselves
 * don't panic; changes can be reverted
Is this good?

-- 
rjbs

--5a720527d7484a05904517687bc40a3c
Content-Type: text/html
Content-Transfer-Encoding: quoted-printable

<!DOCTYPE html><html><head><title></title><style type=3D"text/css">p.Mso=
Normal,p.MsoNoSpacing{margin:0}</style></head><body><div>Thanks for the =
feedback!&nbsp; I think there are couple key things that got called out:=
<br></div><ul><li>a bunch of people want more review<br></li><li>there i=
s skepticism that saying its required will make it happen<br></li><li>no=
body wants to get stuck behind a wall of silence<br></li></ul><div>Part =
of the impetus for this discussion was another point:<br></div><ul><li>n=
obody wants to get stuck when they get a volunteer reviewer who foot-dra=
gs or nit-picks<br></li></ul><div>I also appreciated this comment from C=
raig:<br></div><blockquote type=3D"cite"><div>Traditionally commit bits =
have been handed out when folks got tired of reviewing patches outside o=
f their area of expertise and the patch submitter had a generally good r=
eputation for work that didn't cause trouble.<br></div></blockquote><div=
><br></div><div>Altogether, maybe a structure like this is better:<br></=
div><ul><li>committers should commit their work after an appropriate rev=
iew period spent as a pull request<br></li><li>trivial or obviously-corr=
ect (!) changes may be committed directly (i.e., the appropriate length =
is sometimes zero)<br></li><li>if objections are raised, they need to be=
 addressed (meaning "clearly replied to")</li><li>if the objection is fr=
om a subject matter expert, you need to come to an agreement<br></li><li=
>if the above fails, ping neilb/rjbs/sawyer for adjudication, which I ex=
pect will be quite rare<br></li></ul><div>I think this is very simple, v=
ery close to what we do now, and avoids "I was waiting a very long time"=
 because one is free to say, "Well, I am applying this as I can't get fe=
edback and believe it is correct.<br></div><div><br></div><div>The final=
 things to mention are probably:<br></div><ul><li>for "I think this is r=
ight but want to give a chance for comments", a couple days is probably =
fine<br></li><li>for "this is a significant change that I could really u=
se feedback on," a week or more is probably best, and the PR should prob=
ably be flagged to the list as wanting attention<br></li><li>the later i=
n the release cycle, the stricter we should be with ourselves<br></li><l=
i>don't panic; changes can be reverted<br></li></ul><div>Is this good?<b=
r></div><div><br></div><div>--&nbsp;<br></div><div>rjbs</div><div><br></=
div></body></html>
--5a720527d7484a05904517687bc40a3c--
0
perl
1/16/2021 4:24:19 PM
"Ricardo Signes" <perl.p5p@rjbs.manxome.org> wrote:
:The final things to mention are probably:
: * for "I think this is right but want to give a chance for comments", a couple days is probably fine
: * for "this is a significant change that I could really use feedback on," a week or more is probably best, and the PR should probably be flagged to the list as wanting attention
: * the later in the release cycle, the stricter we should be with ourselves
: * don't panic; changes can be reverted
:Is this good?

Those timescales seem very short to me.

However my attitude is based on my $work experience dealing with high-risk
areas (notably in the handling of 10s or 100s of millions of credit-card
details). I suspect most people's experience has involved programs in which
the potential fallout from errors is much lower impact.

It is not clear to me where errors in perl itself fall on that spectrum:
those using it for high-risk work will likely be taking responsibility
themselves for qualifying any new release of perl for their own application.

But maybe it would be useful if the steering committee could develop and
communicate a clear vision of what degree of risk they're happy to take on
in shipping code that has had lesser review - effectively, which users they
see as a target audience that should be expected to _trust_ a new release.

Hugo
0
hv
1/16/2021 6:22:33 PM
--6fe55e7d782540a2911cb872b1d2373e
Content-Type: text/plain

On Sat, Jan 16, 2021, at 1:22 PM, hv@crypt.org wrote:
> "Ricardo Signes" <perl.p5p@rjbs.manxome.org> wrote:
> :The final things to mention are probably:
> : * for "I think this is right but want to give a chance for comments", a couple days is probably fine
> : * for "this is a significant change that I could really use feedback on," a week or more is probably best, and the PR should probably be flagged to the list as wanting attention
> : * the later in the release cycle, the stricter we should be with ourselves
> : * don't panic; changes can be reverted
> :Is this good?
> 
> Those timescales seem very short to me.

Briefly reply now because I'm worried I'll forget later:

I agree, they feel short to me, but I know that's because I'm considering them in the context of paid work.  In paid work, one's colleagues have (say) forty hour-long timeslices in a week, and allocating a few for you is not an enormous cost.  In work on the Perl core, very few of us have that many timeslices available for Perl work, and using it on review will feel, for most people, like second-order work that helps some other volunteer achieve their goals, rather than one's own goals.

This means, perforce, that we need to limit the things that can be blocked by non-review.  As you say, it's a question of risk.  I want to acknowledge that commit bits have been given to people who we believe are good at policing their own risk.

(Also, having a lot more active, expert committers would allow stricter requirements, but we're not there at the moment.)

-- 
rjbs
--6fe55e7d782540a2911cb872b1d2373e
Content-Type: text/html
Content-Transfer-Encoding: quoted-printable

<!DOCTYPE html><html><head><title></title><style type=3D"text/css">p.Mso=
Normal,p.MsoNoSpacing{margin:0}</style></head><body><div>On Sat, Jan 16,=
 2021, at 1:22 PM,&nbsp;<a href=3D"mailto:hv@crypt.org">hv@crypt.org</a>=
 wrote:<br></div><blockquote type=3D"cite" id=3D"qt" style=3D""><div>"Ri=
cardo Signes" &lt;<a href=3D"mailto:perl.p5p@rjbs.manxome.org">perl.p5p@=
rjbs.manxome.org</a>&gt; wrote:<br></div><div>:The final things to menti=
on are probably:<br></div><div>: * for "I think this is right but want t=
o give a chance for comments", a couple days is probably fine<br></div><=
div>: * for "this is a significant change that I could really use feedba=
ck on," a week or more is probably best, and the PR should probably be f=
lagged to the list as wanting attention<br></div><div>: * the later in t=
he release cycle, the stricter we should be with ourselves<br></div><div=
>: * don't panic; changes can be reverted<br></div><div>:Is this good?<b=
r></div><div><br></div><div>Those timescales seem very short to me.<br><=
/div></blockquote><div><br></div><div>Briefly reply now because I'm worr=
ied I'll forget later:<br></div><div><br></div><div>I agree, they feel s=
hort to me, but I know that's because I'm considering them in the contex=
t of paid work.&nbsp; In paid work, one's colleagues have (say) forty ho=
ur-long timeslices in a week, and allocating a few for you is not an eno=
rmous cost.&nbsp; In work on the Perl core, very few of us have that man=
y timeslices available for Perl work, and using it on review will feel, =
for most people, like second-order work that helps some other volunteer =
achieve their goals, rather than one's own goals.<br></div><div><br></di=
v><div>This means, perforce, that we need to limit the things that can b=
e blocked by non-review.&nbsp; As you say, it's a question of risk.&nbsp=
; I want to acknowledge that commit bits have been given to people who w=
e believe are good at policing their own risk.<br></div><div><br></div><=
div>(Also, having a lot more active, expert committers would allow stric=
ter requirements, but we're not there at the moment.)</div><div><br></di=
v><div>--&nbsp;<br></div><div>rjbs</div></body></html>
--6fe55e7d782540a2911cb872b1d2373e--
0
perl
1/16/2021 6:56:04 PM
Reply: