lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Robert Muir <rcm...@gmail.com>
Subject Re: Commit / Code Review Policy
Date Thu, 05 Dec 2019 13:21:37 GMT
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
>
>

Mime
View raw message