lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gus Heck <gus.h...@gmail.com>
Subject Re: Commit / Code Review Policy
Date Thu, 05 Dec 2019 16:47:25 GMT
And a link to guidelines on what goes where....

On Thu, Dec 5, 2019 at 10:49 AM Jan Høydahl <jan.asf@cominvent.com> wrote:

> The SIP template should have a question that each proposal MUST answer:
>
> “Describe your consideration of what goes in solr-core and what goes in
> packages or contrib.”
>
> Jan Høydahl
>
> 5. des. 2019 kl. 14:22 skrev Robert Muir <rcmuir@gmail.com>:
>
> 
> Fine, here's my SIP process.
>
> You can't add one feature unless you remove two others.
>
> Very simple and works.
>
> On Thu, Dec 5, 2019 at 4:11 AM Jan Høydahl <jan.asf@cominvent.com> wrote:
>
>> Let’s keep this thread about code review guidelines, not about feature
>> removal, please. And be concrete with proposals for re-wording Davids
>> proposal instead of sidetracking.
>> As David said, we need to do both. I think the SIP process for new
>> features and APIs may be a far better way than some hard «feature freeze».
>>
>> Jan
>>
>> > 4. des. 2019 kl. 22:49 skrev Noble Paul <noble.paul@gmail.com>:
>> >
>> >> I don't think this has anything to do with code review: it has to do
>> with people just piling in features, but not taking the time to do any
>> janitorial work or remove old features that shouldn't be there anymore (I
>> AM LOOKING AT YOU HDFS)
>> >
>> > 100 %. If there is one problem with Solr today, it is feature bloat.
>> > We need to trim down Solr by atleast 50%
>> >
>> > What we need to do urgently is to create a white list of essential
>> > features and eliminate others. We are just getting crushed by the
>> > amount of code we have in Solr. We don't have so many people who can
>> > actively maintain such a large codebase
>> >
>> > On Thu, Dec 5, 2019 at 7:34 AM David Smiley <david.w.smiley@gmail.com>
>> wrote:
>> >>
>> >> Mike D.,
>> >>  I loved your response, especially for researching what other projects
>> do!
>> >>
>> >> ... more responses within...
>> >>
>> >> On Tue, Dec 3, 2019 at 2:42 PM Mike Drob <mdrob@apache.org> wrote:
>> >>>
>> >>> I'm coming late into this thread because a lot of the discussion
>> happened while I was offline, so some of the focus has shifted, but I'll
>> try to address a few topics that were brought up earlier anyway. In an
>> effort to be brief, I'll try to summarize sentiments rather than addressing
>> specific quotes - if I mischaracterize something please let me know!
>> >>>
>> >>> First, I don't believe that RTC requires consensus voting with 3
>> reviews per commit or anything nearly so burdensome. A brief survey of
>> other Apache projects shows that most on RTC only need a single review, and
>> also can include exceptions for trivial changes like we are discussing
>> here. If we're trying to find a compromise position, I personally would
>> prefer to be more on the RTC side because I think it's a more responsible
>> place to be for a project that backs billions of dollars of revenue across
>> hundreds of companies annually.
>> >>>
>> >>> Spark is pretty strict RTC, but with such a volume of contributions
>> it may be the only option sustainable for them.
>> https://spark.apache.org/contributing.html
>> >>> HBase requires a review and has an exception for trivial patches -
>> http://hbase.apache.org/book.html#_review
>> >>> Yetus requires reviews, but allows binding review from
>> non-committers, and has a no review expiration.
>> https://s.apache.org/mi0r8 - there's a lot of other good discussion
>> there too.
>> >>> Zookeeper requires a minimum one day waiting period on all code
>> change, but does not require explicit reviews. -
>> https://zookeeper.apache.org/bylaws.html
>> >>>
>> >>> One piece I'll echo from the Yetus discussion is that if we have a
>> habit of review, then we're less likely to get stagnant patches, and we're
>> also more likely to engage with non-committer contributions. If we don't
>> have that habit of reviews, then patches do get stale, and
>> trust/self-enforcement becomes the only sustainable way forward.
>> >>>
>> >>> A second point that I'm also concerned about is the idea that we
>> don't want JIRA issues or CHANGES entries for trivial or even minor
>> patches. This has a huge impact on potential contributors and their
>> accessibility to the project. If contributors aren't credited for all of
>> their work, then it makes it harder for them to reach invitation as a
>> committer. As a personal example, a lot of my changes were around logging
>> and error handling, which are minor on your list but fairly important for a
>> supporter in aggregate. If the community signalled that the work was less
>> valuable, less visible, and less likely to be accepted (each of which can
>> follow from the previous I think) then I don't know that I would have been
>> motivated to try and contribute those issues.
>> >>
>> >>
>> >> Our CHANGES.txt tries to simultaneously be a useful document in
>> informing users (and us) of what was changed for what issue that we might
>> actually care about, and *also* give kudos to contributors.  There is a lot
>> of noise in there, as it is.  If hypothetically a contributor files a JIRA
>> issue with minor/trivial priority, then maybe a git author tag is enough?
>> Or if not then maybe adding a special section in the CHANGES.txt for a
>> special thanks to contributors of unspecified issues?
>> >>
>> >>>
>> >>> To the point about security issues, that's something that should
>> probably be addressed explicitly on the security/private list, and it
>> absolutely needs review if only so that other developers can learn and
>> avoid those issues again. That's where the power of community is really
>> important, and I don't expect issues like that to sit around with a patch
>> waiting for very long anyway.
>> >>>
>> >>> Overall, I think following in the Yetus or ZK model with a 72 hour
>> timeout is a reasonable compromise, especially because a hard shift from
>> CTR to RTC would need a corresponding culture shift that may not happen
>> immediately.
>> >>
>> >>
>> >> Yes I agree.  Can you suggest a proposed guideline (or perhaps actual
>> policy?  hmm)?  Today our guidelines don't quite have this rule, which thus
>> allows a committer to commit a major change immediately without a review
>> (ouch!).  Surely we don't *actually* want to allow that.
>> >>
>> >>>
>> >>>
>> >>> Mike
>> >
>> >
>> >
>> > --
>> > -----------------------------------------------------
>> > Noble Paul
>> >
>> > ---------------------------------------------------------------------
>> > To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
>> > For additional commands, e-mail: dev-help@lucene.apache.org
>> >
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
>> For additional commands, e-mail: dev-help@lucene.apache.org
>>
>>

-- 
http://www.needhamsoftware.com (work)
http://www.the111shift.com (play)

Mime
View raw message