You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by David Smiley <da...@gmail.com> on 2020/02/07 15:14:37 UTC

Re: Commit / Code Review Policy

I neglected to try and close up this long thread on the subject of code
review guidance.  In the project's board report to the ASF, I asked for
help; Daniel Ruggeri (an ASF VP) graciously volunteered.  He's on the "To"
line to my message here; he's not a member of our dev list.

Daniel:

Thanks in advance for any assistance / guidance you have to offer.

I suggest first reading through this thread:
https://lucene.472066.n3.nabble.com/Commit-Code-Review-Policy-td4452928.html
I find Nabble much easier to navigate than the ASF official archive, but
that's here if you prefer:
http://mail-archives.apache.org/mod_mbox/lucene-dev/201911.mbox/%3cCABEwPvET1u4xGxQadddheS+uyBEXMuw33C8fbS5BZDLzULCRag@mail.gmail.com%3e
  It starts November 26th and ended December 8th.

The second thing is my proposal document stored in Confluence.  I have yet
to rename it as I said I would but I'll let it be for a little bit so the
links continue to work for the moment:
https://cwiki.apache.org/confluence/display/LUCENE/Commit+Policy+-+DRAFT
Everything from "Tests" heading onwards has a "[PENDING DISCUSSION]" notice
and can be ignored for the time being.

~ David

Re: Commit / Code Review Policy

Posted by David Smiley <da...@gmail.com>.
On Wed, Mar 4, 2020 at 8:36 AM Daniel Ruggeri <dr...@apache.org> wrote:

> I definitely reviewed the email thread, replies, and Confluence wiki
> document. In my interpretation of the discussion, a few things stood out:
> - There were some synchronous discussions that occurred off the list,
> which can lead to accidentally excluding important community voices from
> the conversation. By bringing the discussion back to the list before
> attempting to make a decision, the critical levels of transparency are
> maintained. This is definitely A Good Thing.
>

I agree 100% in what you said.  I thought about this principle as I
conducted my business (off-list and on-list).  I'm frustrated that not
everyone agreed; I really do care!  If we are so fearful of crossing a line
here, then we end up limiting ourselves to excluding opportunities to see
each other and hear each other's voices, and that would be shame!  A lot
can be lost in purely text chat in ascertaining emotion / intent.  Seeing
and hearing each other can help diffuse possible conflict and furthermore
it enriches our relationships in subtle ways.  Do you agree?  Do other
projects have written guidelines that we can adopt to state this more
concretely so we don't cross a line?


> - I perceived a lot of constructive feedback on the initial draft which
> appears (to me) to have brought the draft much closer to a document that
> represents consensus. Said another way, I don't detect conflict per se
> (please correct me if I'm wrong)
>

I agree; it was all constructive.


> - I think the feedback (and subsequent incorporation of that feedback into
> the guidelines) about exception cases like doc fixes and "small" changes is
> brilliant. On the httpd project, as a parallel example, we have a RTC
> policy for release branches except for docs - which are CTR. Indeed, a
> "docs committer" is the same as an "httpd committer"... we don't separate
> privileges because this social contract has worked well for 2 decades.
>

Thanks!  Documenting this is important to me as too often I see too much
ceremony for such trivial stuff which in turn leads to reduced
contributions from me because I don't want to go through that ceremony when
I don't think it's warranted.


> - The current content of the document seems reasonable given the current
> environment. One thing that is unclear to me based on a quick browsing of
> the docs is what the branching strategy is for the project. What I gather
> is that the master branch is the release branch and features are added to
> master or merged in from short-lived feature branches. It'd be helpful to
> clarify this.
>

True; we should probably state the strategy _somewhere_; maybe this
document is where.  Although such info wouldn't really be
guidance/advice so I'm not sure it goes on the document.  We branch off
from our main/dev branches to do releases, and so I wouldn't call "master"
and "branch_8x" release branches since we do have actual release branches
created at feature-freeze time.


> - The proposed document draws additional parallels to things that work for
> httpd. Often times, we will share patches to the list for feedback before
> commits (similar to the Jira tickets proposed). We also operate trunk as
> CTR, but require 3 +1 backport votes to the long-lived release branches.
> This proposal has a similar net effect: bits that get released are
> (generally) intended to be "actively" reviewed before commit. It does fall
> short of requiring a consensus-style vote for release branches as httpd
> does... but that's OK.
>

Hmmm.  What we do is have the release manager be a required gatekeeper as
to what may be committed to a release branch (yay/nay decision maker), and
we know basically what the general qualifications are which are repeated at
every feature freeze.  This document would be a good place to state them;
good idea.


>
> All told, this proposal seems quite reasonable to me because it has been
> discussed by the community, feedback has been incorporated, and the content
> of the proposal seems totally doable. That said, I'm open to other
> inquiries if there's something an external perspective can provide. How can
> I help?
>

I suppose we're in good shape then.  I have one question that you more than
others could give an authoritative answer to:  Do projects really choose
RTC: http://www.apache.org/foundation/glossary.html#ReviewThenCommit given
the onerous requirements?  It's hard for me to imagine it being workable
unless there were lots of active committers and if the volume of change is
low and if the changes are potentially sensitive (e.g. for encryption).  If
the Lucene project hypothetically wanted to insist on at least one +1 from
someone, would we not be able to call this RTC because it doesn't meet the
definition above?  I suspect in truth we can define our policy as we wish.


>
> [1] https://whimsy.apache.org/board/minutes/Lucene.html
>
> P.S.
> Nabble, mbox, etc... they're OK, but have you tried Ponymail?
>
> https://lists.apache.org/x/thread.html/770ef83a8ed3a3d5d161c4a2cd812b4bdde47464d439c09ec31164dd@%3Cdev.lucene.apache.org%3E
> I'm a convert, for sure!


Thanks for showing me Ponymail!  It's *way* better than whatever that
interface is for mod_mbox   URLs.  Our project website should replace all
such links to Ponymail.

~ David

Re: Commit / Code Review Policy

Posted by David Smiley <da...@gmail.com>.
(Renamed; no other changes)
https://cwiki.apache.org/confluence/display/LUCENE/Commit+Process+Guidelines

~ David Smiley
Apache Lucene/Solr Search Developer
http://www.linkedin.com/in/davidwsmiley


On Wed, Mar 4, 2020 at 8:36 AM Daniel Ruggeri <dr...@apache.org> wrote:

> Hi, all;
> I could have *sworn* I replied... but could find no record in my sent
> folder when David dutifully followed up with a ping. *facepalm*
>
> Yes, I volunteered to participate and help where I can as a member of The
> ASF board, VP Fundraising, ComDev participant, or whatever hat I might be
> able to bring with me. Of course, I bear no special credentials here... we
> all hang out as volunteers. Reviewing the most recent board report[1], I
> detected:
> > We're discussing how to semi-require code reviews by defining project
> guidelines. We invite a board member to help/advice how to navigate this
> balance.
> So, yeah, here I am :-) My primary interest is to be sure we're
> approaching the discussion with community as our primary lens.
>
> I definitely reviewed the email thread, replies, and Confluence wiki
> document. In my interpretation of the discussion, a few things stood out:
> - There were some synchronous discussions that occurred off the list,
> which can lead to accidentally excluding important community voices from
> the conversation. By bringing the discussion back to the list before
> attempting to make a decision, the critical levels of transparency are
> maintained. This is definitely A Good Thing.
> - I perceived a lot of constructive feedback on the initial draft which
> appears (to me) to have brought the draft much closer to a document that
> represents consensus. Said another way, I don't detect conflict per se
> (please correct me if I'm wrong)
> - I think the feedback (and subsequent incorporation of that feedback into
> the guidelines) about exception cases like doc fixes and "small" changes is
> brilliant. On the httpd project, as a parallel example, we have a RTC
> policy for release branches except for docs - which are CTR. Indeed, a
> "docs committer" is the same as an "httpd committer"... we don't separate
> privileges because this social contract has worked well for 2 decades.
> - The current content of the document seems reasonable given the current
> environment. One thing that is unclear to me based on a quick browsing of
> the docs is what the branching strategy is for the project. What I gather
> is that the master branch is the release branch and features are added to
> master or merged in from short-lived feature branches. It'd be helpful to
> clarify this.
> - The proposed document draws additional parallels to things that work for
> httpd. Often times, we will share patches to the list for feedback before
> commits (similar to the Jira tickets proposed). We also operate trunk as
> CTR, but require 3 +1 backport votes to the long-lived release branches.
> This proposal has a similar net effect: bits that get released are
> (generally) intended to be "actively" reviewed before commit. It does fall
> short of requiring a consensus-style vote for release branches as httpd
> does... but that's OK.
>
> All told, this proposal seems quite reasonable to me because it has been
> discussed by the community, feedback has been incorporated, and the content
> of the proposal seems totally doable. That said, I'm open to other
> inquiries if there's something an external perspective can provide. How can
> I help?
>
>
> [1] https://whimsy.apache.org/board/minutes/Lucene.html
>
> P.S.
> Nabble, mbox, etc... they're OK, but have you tried Ponymail?
>
> https://lists.apache.org/x/thread.html/770ef83a8ed3a3d5d161c4a2cd812b4bdde47464d439c09ec31164dd@%3Cdev.lucene.apache.org%3E
> I'm a convert, for sure!
> --
> Daniel Ruggeri
> Director, VP Fundraising, member, httpd PMC
> The Apache Software Foundation
>
> On February 7, 2020 9:14:37 AM CST, David Smiley <da...@gmail.com>
> wrote:
>>
>> I neglected to try and close up this long thread on the subject of code
>> review guidance.  In the project's board report to the ASF, I asked for
>> help; Daniel Ruggeri (an ASF VP) graciously volunteered.  He's on the "To"
>> line to my message here; he's not a member of our dev list.
>>
>> Daniel:
>>
>> Thanks in advance for any assistance / guidance you have to offer.
>>
>> I suggest first reading through this thread:
>>
>> https://lucene.472066.n3.nabble.com/Commit-Code-Review-Policy-td4452928.html
>> I find Nabble much easier to navigate than the ASF official archive, but
>> that's here if you prefer:
>> http://mail-archives.apache.org/mod_mbox/lucene-dev/201911.mbox/%3cCABEwPvET1u4xGxQadddheS+uyBEXMuw33C8fbS5BZDLzULCRag@mail.gmail.com%3e
>>   It starts November 26th and ended December 8th.
>>
>> The second thing is my proposal document stored in Confluence.  I have
>> yet to rename it as I said I would but I'll let it be for a little bit so
>> the links continue to work for the moment:
>> https://cwiki.apache.org/confluence/display/LUCENE/Commit+Policy+-+DRAFT
>> Everything from "Tests" heading onwards has a "[PENDING DISCUSSION]"
>> notice and can be ignored for the time being.
>>
>> ~ David
>>
>

Re: Commit / Code Review Policy

Posted by Daniel Ruggeri <dr...@apache.org>.
Hi, all;
   I could have *sworn* I replied... but could find no record in my sent folder when David dutifully followed up with a ping. *facepalm*

   Yes, I volunteered to participate and help where I can as a member of The ASF board, VP Fundraising, ComDev participant, or whatever hat I might be able to bring with me. Of course, I bear no special credentials here... we all hang out as volunteers. Reviewing the most recent board report[1], I detected:
> We're discussing how to semi-require code reviews by defining project guidelines. We invite a board member to help/advice how to navigate this balance. 
So, yeah, here I am :-) My primary interest is to be sure we're approaching the discussion with community as our primary lens.

   I definitely reviewed the email thread, replies, and Confluence wiki document. In my interpretation of the discussion, a few things stood out:
- There were some synchronous discussions that occurred off the list, which can lead to accidentally excluding important community voices from the conversation. By bringing the discussion back to the list before attempting to make a decision, the critical levels of transparency are maintained. This is definitely A Good Thing.
- I perceived a lot of constructive feedback on the initial draft which appears (to me) to have brought the draft much closer to a document that represents consensus. Said another way, I don't detect conflict per se (please correct me if I'm wrong)
- I think the feedback (and subsequent incorporation of that feedback into the guidelines) about exception cases like doc fixes and "small" changes is brilliant. On the httpd project, as a parallel example, we have a RTC policy for release branches except for docs - which are CTR. Indeed, a "docs committer" is the same as an "httpd committer"... we don't separate privileges because this social contract has worked well for 2 decades.
- The current content of the document seems reasonable given the current environment. One thing that is unclear to me based on a quick browsing of the docs is what the branching strategy is for the project. What I gather is that the master branch is the release branch and features are added to master or merged in from short-lived feature branches. It'd be helpful to clarify this.
- The proposed document draws additional parallels to things that work for httpd. Often times, we will share patches to the list for feedback before commits (similar to the Jira tickets proposed). We also operate trunk as CTR, but require 3 +1 backport votes to the long-lived release branches. This proposal has a similar net effect: bits that get released are (generally) intended to be "actively" reviewed before commit. It does fall short of requiring a consensus-style vote for release branches as httpd does... but that's OK.

All told, this proposal seems quite reasonable to me because it has been discussed by the community, feedback has been incorporated, and the content of the proposal seems totally doable. That said, I'm open to other inquiries if there's something an external perspective can provide. How can I help?


[1] https://whimsy.apache.org/board/minutes/Lucene.html

P.S.
Nabble, mbox, etc... they're OK, but have you tried Ponymail?
https://lists.apache.org/x/thread.html/770ef83a8ed3a3d5d161c4a2cd812b4bdde47464d439c09ec31164dd@%3Cdev.lucene.apache.org%3E
I'm a convert, for sure!
-- 
Daniel Ruggeri
Director, VP Fundraising, member, httpd PMC
The Apache Software Foundation

On February 7, 2020 9:14:37 AM CST, David Smiley <da...@gmail.com> wrote:
>I neglected to try and close up this long thread on the subject of code
>review guidance.  In the project's board report to the ASF, I asked for
>help; Daniel Ruggeri (an ASF VP) graciously volunteered.  He's on the
>"To"
>line to my message here; he's not a member of our dev list.
>
>Daniel:
>
>Thanks in advance for any assistance / guidance you have to offer.
>
>I suggest first reading through this thread:
>https://lucene.472066.n3.nabble.com/Commit-Code-Review-Policy-td4452928.html
>I find Nabble much easier to navigate than the ASF official archive,
>but
>that's here if you prefer:
>http://mail-archives.apache.org/mod_mbox/lucene-dev/201911.mbox/%3cCABEwPvET1u4xGxQadddheS+uyBEXMuw33C8fbS5BZDLzULCRag@mail.gmail.com%3e
>  It starts November 26th and ended December 8th.
>
>The second thing is my proposal document stored in Confluence.  I have
>yet
>to rename it as I said I would but I'll let it be for a little bit so
>the
>links continue to work for the moment:
>https://cwiki.apache.org/confluence/display/LUCENE/Commit+Policy+-+DRAFT
>Everything from "Tests" heading onwards has a "[PENDING DISCUSSION]"
>notice
>and can be ignored for the time being.
>
>~ David