> 26. nov. 2019 kl. 06:34 skrev David Smiley <[EMAIL PROTECTED]>:
>
> Last Wednesday at a Solr committers meeting, there was general agreement in attendance to raise the bar for commit permission to require another's consent, which might not have entailed a review of the code.  I volunteered to draft a proposal.  Other things distracted me but I'm finally thinking of this task now.  *This email is NOT the proposal*.
>
> I was about to write something from scratch when I discovered we already have some internal documentation on a commit policy that is both reasonably well written/composed and the actual policy/information is pretty good -- kudos to the mystery author!
>
> https://cwiki.apache.org/confluence/display/SOLR/CommitPolicy <https://cwiki.apache.org/confluence/display/SOLR/CommitPolicy>
>
> I'd prefer we have one "Commit Policy" document for Lucene/Solr and only call out Solr specifics when applicable.  This is easier to maintain and is in line with the joint-ness of Lucene TLP.  So I think it should move to the Lucene cwiki.  Granted there is a possibility this kind of content might move into our source control somewhere but that possibility is a subject for another day.
>
> I plan to copy this to Lucene, mark as PROPOSAL and then make some large edits.  The diff will probably be kinda unrecognizable despite it being in nice shape now.  A "Commit Policy" is more broad that a "Code Review Policy"; it could cover a variety of things.  For example when to commit without even filing a JIRA issue, which I think is worth mentioning.  It should probably also cover Git considerations like merge vs rebase, and multiple commits vs squashing.  Maybe we should also cover when to bother adding to CHANGES.txt and "via"?  Probably commit message requirements.  Snowballing scope :-). Probably not JIRA metadata as it's not part of the commit to be part of a commit policy, but _somewhere_ that's needed.  I'm not sure I want to  sign up for all that now but at least for the code review subject.
>
> ~ David Smiley
> Apache Lucene/Solr Search Developer
> http://www.linkedin.com/in/davidwsmiley <http://www.linkedin.com/in/davidwsmiley>

>
> Last Wednesday at a Solr committers meeting, there was general agreement in attendance to raise the bar for commit permission to require another's consent, which might not have entailed a review of the code.  I volunteered to draft a proposal.  Other things distracted me but I'm finally thinking of this task now.  *This email is NOT the proposal*.
>
> I was about to write something from scratch when I discovered we already have some internal documentation on a commit policy that is both reasonably well written/composed and the actual policy/information is pretty good -- kudos to the mystery author!
>
> https://cwiki.apache.org/confluence/display/SOLR/CommitPolicy
>
> I'd prefer we have one "Commit Policy" document for Lucene/Solr and only call out Solr specifics when applicable.  This is easier to maintain and is in line with the joint-ness of Lucene TLP.  So I think it should move to the Lucene cwiki.  Granted there is a possibility this kind of content might move into our source control somewhere but that possibility is a subject for another day.
>
> I plan to copy this to Lucene, mark as PROPOSAL and then make some large edits.  The diff will probably be kinda unrecognizable despite it being in nice shape now.  A "Commit Policy" is more broad that a "Code Review Policy"; it could cover a variety of things.  For example when to commit without even filing a JIRA issue, which I think is worth mentioning.  It should probably also cover Git considerations like merge vs rebase, and multiple commits vs squashing.  Maybe we should also cover when to bother adding to CHANGES.txt and "via"?  Probably commit message requirements.  Snowballing scope :-). Probably not JIRA metadata as it's not part of the commit to be part of a commit policy, but _somewhere_ that's needed.  I'm not sure I want to  sign up for all that now but at least for the code review subject.
>
> ~ David Smiley
> Apache Lucene/Solr Search Developer
> http://www.linkedin.com/in/davidwsmiley

> This document looks reasonable to me and a good description of the way
> changes get merged today. Something it says between the lines and that
> is the most important bit to me is that this isn't really a policy but
> rather a set of guidelines, and that we trust each other to do the
> right thing. Maybe we could better reflect this in the name, e.g.
> "Commit/Merging guidelines".
>
> On Tue, Nov 26, 2019 at 6:34 AM David Smiley <[EMAIL PROTECTED]>
> wrote:
> >
> > Last Wednesday at a Solr committers meeting, there was general agreement
> in attendance to raise the bar for commit permission to require another's
> consent, which might not have entailed a review of the code.  I volunteered
> to draft a proposal.  Other things distracted me but I'm finally thinking
> of this task now.  *This email is NOT the proposal*.
> >
> > I was about to write something from scratch when I discovered we already
> have some internal documentation on a commit policy that is both reasonably
> well written/composed and the actual policy/information is pretty good --
> kudos to the mystery author!
> >
> > https://cwiki.apache.org/confluence/display/SOLR/CommitPolicy
> >
> > I'd prefer we have one "Commit Policy" document for Lucene/Solr and only
> call out Solr specifics when applicable.  This is easier to maintain and is
> in line with the joint-ness of Lucene TLP.  So I think it should move to
> the Lucene cwiki.  Granted there is a possibility this kind of content
> might move into our source control somewhere but that possibility is a
> subject for another day.
> >
> > I plan to copy this to Lucene, mark as PROPOSAL and then make some large
> edits.  The diff will probably be kinda unrecognizable despite it being in
> nice shape now.  A "Commit Policy" is more broad that a "Code Review
> Policy"; it could cover a variety of things.  For example when to commit
> without even filing a JIRA issue, which I think is worth mentioning.  It
> should probably also cover Git considerations like merge vs rebase, and
> multiple commits vs squashing.  Maybe we should also cover when to bother
> adding to CHANGES.txt and "via"?  Probably commit message requirements.
> Snowballing scope :-). Probably not JIRA metadata as it's not part of the
> commit to be part of a commit policy, but _somewhere_ that's needed.  I'm
> not sure I want to  sign up for all that now but at least for the code
> review subject.
> >
> > ~ David Smiley
> > Apache Lucene/Solr Search Developer
> > http://www.linkedin.com/in/davidwsmiley
>
>
>
> --
> Adrien
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
>
> --

> +1
>
> I am generally wary of such proposals since they tend to impose hard
> processes in the places where trust should be dominant instead.
>
> Apart from that, LGTM
>
> On Tue, 26 Nov 2019 at 14:46, Adrien Grand <[EMAIL PROTECTED]> wrote:
>
>> This document looks reasonable to me and a good description of the way
>> changes get merged today. Something it says between the lines and that
>> is the most important bit to me is that this isn't really a policy but
>> rather a set of guidelines, and that we trust each other to do the
>> right thing. Maybe we could better reflect this in the name, e.g.
>> "Commit/Merging guidelines".
>>
>> On Tue, Nov 26, 2019 at 6:34 AM David Smiley <[EMAIL PROTECTED]>
>> wrote:
>> >
>> > Last Wednesday at a Solr committers meeting, there was general
>> agreement in attendance to raise the bar for commit permission to require
>> another's consent, which might not have entailed a review of the code.  I
>> volunteered to draft a proposal.  Other things distracted me but I'm
>> finally thinking of this task now.  *This email is NOT the proposal*.
>> >
>> > I was about to write something from scratch when I discovered we
>> already have some internal documentation on a commit policy that is both
>> reasonably well written/composed and the actual policy/information is
>> pretty good -- kudos to the mystery author!
>> >
>> > https://cwiki.apache.org/confluence/display/SOLR/CommitPolicy
>> >
>> > I'd prefer we have one "Commit Policy" document for Lucene/Solr and
>> only call out Solr specifics when applicable.  This is easier to maintain
>> and is in line with the joint-ness of Lucene TLP.  So I think it should
>> move to the Lucene cwiki.  Granted there is a possibility this kind of
>> content might move into our source control somewhere but that possibility
>> is a subject for another day.
>> >
>> > I plan to copy this to Lucene, mark as PROPOSAL and then make some
>> large edits.  The diff will probably be kinda unrecognizable despite it
>> being in nice shape now.  A "Commit Policy" is more broad that a "Code
>> Review Policy"; it could cover a variety of things.  For example when to
>> commit without even filing a JIRA issue, which I think is worth
>> mentioning.  It should probably also cover Git considerations like merge vs
>> rebase, and multiple commits vs squashing.  Maybe we should also cover when
>> to bother adding to CHANGES.txt and "via"?  Probably commit message
>> requirements.  Snowballing scope :-). Probably not JIRA metadata as it's
>> not part of the commit to be part of a commit policy, but _somewhere_
>> that's needed.  I'm not sure I want to  sign up for all that now but at
>> least for the code review subject.
>> >
>> > ~ David Smiley
>> > Apache Lucene/Solr Search Developer
>> > http://www.linkedin.com/in/davidwsmiley
>>
>>
>>
>> --
>> Adrien
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [EMAIL PROTECTED]
>> For additional commands, e-mail: [EMAIL PROTECTED]
>>
>> --
> Regards,
>
> Atri
> Apache Concerted
>

>
> Last Wednesday at a Solr committers meeting, there was general agreement in attendance to raise the bar for commit permission to require another's consent, which might not have entailed a review of the code.  I volunteered to draft a proposal.  Other things distracted me but I'm finally thinking of this task now.  *This email is NOT the proposal*.
>
> I was about to write something from scratch when I discovered we already have some internal documentation on a commit policy that is both reasonably well written/composed and the actual policy/information is pretty good -- kudos to the mystery author!
>
> https://cwiki.apache.org/confluence/display/SOLR/CommitPolicy
>
> I'd prefer we have one "Commit Policy" document for Lucene/Solr and only call out Solr specifics when applicable.  This is easier to maintain and is in line with the joint-ness of Lucene TLP.  So I think it should move to the Lucene cwiki.  Granted there is a possibility this kind of content might move into our source control somewhere but that possibility is a subject for another day.
>
> I plan to copy this to Lucene, mark as PROPOSAL and then make some large edits.  The diff will probably be kinda unrecognizable despite it being in nice shape now.  A "Commit Policy" is more broad that a "Code Review Policy"; it could cover a variety of things.  For example when to commit without even filing a JIRA issue, which I think is worth mentioning.  It should probably also cover Git considerations like merge vs rebase, and multiple commits vs squashing.  Maybe we should also cover when to bother adding to CHANGES.txt and "via"?  Probably commit message requirements.  Snowballing scope :-). Probably not JIRA metadata as it's not part of the commit to be part of a commit policy, but _somewhere_ that's needed.  I'm not sure I want to  sign up for all that now but at least for the code review subject.
>
> ~ David Smiley
> Apache Lucene/Solr Search Developer
> http://www.linkedin.com/in/davidwsmiley

> The commit policy / guideline document is basically 95% there and I don't
> want to wait longer to get input.
> https://cwiki.apache.org/confluence/display/LUCENE/Commit+Policy+-+DRAFT
>
> If you log-in, you can comment on the document in-line as Jan has already
> done.  Such feedback is good for details.  For more substantive or high
> level feedback, this email thread probably makes more sense.
>
> The policy/guideline document insists on reviews but gives broad
> exceptions for reviews and defines a very low bar for reviews -- basically
> mere "approval" from *anyone* and that didn't necessarily look at the
> code.  Yet this is a higher bar than today.
>
> Also, I hope this is not controversial but I want the same definition of
> minor/trival matters to be used for (A) when a JIRA issue is not needed
> either, and (B) not bothering with a CHANGES.txt entry.  I observe that
> today we seemingly have a JIRA issue for *everything*, and I find that
> onerous and is yet another barrier for contributors of such small matters.
> For example https://issues.apache.org/jira/browse/SOLR-13926 which just
> adds javadocs.  Also I think we add too many items to CHANGES.txt... lots
> of people read this and it's a collective waste of our time IMO to mention
> that some test was fixed.
>
> All feedback is very welcome!
>
> ~ David
>

> Last Wednesday at a Solr committers meeting, there was general agreement...
>
> I'd prefer we have one "Commit Policy" document for Lucene/Solr and only
> call out Solr specifics when applicable.  This is easier to maintain and is
> in line with the joint-ness of Lucene TLP.  So I think it should move to
> the Lucene cwiki.  Granted there is a possibility this kind of content
> might move into our source control somewhere but that possibility is a
> subject for another day.
>
>

> -1 ... you even went so far as to discourage lucene committers from
> attending that meeting, and now its turned around as if its consensus
> everywhere and should be applied to lucene too?
>
> I don't think changing things to review-then-commit will help.

>
> On 11/30/2019 7:39 AM, Robert Muir wrote:
> > -1 ... you even went so far as to discourage lucene committers from
> > attending that meeting, and now its turned around as if its consensus
> > everywhere and should be applied to lucene too?
> >
> > I don't think changing things to review-then-commit will help.
>
> I did attend the meeting, and I think committers who concentrate on
> Lucene should not be discouraged from attending any similar future
> meetings.  Discussions in a meeting about Solr are not very likely to
> dive down that far into the inner workings, but even if they don't, some
> Lucene people might find the the talk about Solr to be interesting, and
> having their perspective available would not be a bad thing.
>
> I'm very wary of making an official change to review-then-commit.  I
> fully support the ideas that went into the proposal, but I think making
> it mandatory for all commits is going to really slow things down and
> cause some problems.  It's not the way most Apache projects work, and it
> makes it a LOT harder to do quick sanity edits like fixing spelling errors.
>
> As much as I really like the idea of more frequent reviews, I think that
> review requirements should be informal.
>
> Most of the time a committer will know whether the changes they are
> working on fall into a "major" or "minor" category.  When it's leaning
> more towards major, I think most of us will agree that a few extra
> eyeballs looking for gotchas is a really good idea.  TLDR note:  The
> number of lines in a change is sometimes NOT an indicator of how major
> the change is.
>
> I certainly want to seek a "looks good to me" on changes I make which
> have any significant impact.  For some of the ideas I have, if those
> ever reach the implementation phase, I think I'd want even more
> assurance that I'm not doing it wrong.  I can pledge that I will seek
> review for non-trivial changes.  I wonder if there is wording we can add
> to any official project rulebook to make such a thing mandatory, without
> a full switch.
>
> I think that a full switch to review-then-commit (RTC) would be the
> wrong thing to do.  I think it would lead to a lot of dissent within the
> project, encouraging rivalries and cliques within the project.
>
> If RTC is preferred by a significant majority, I will work within that
> paradigm ... but I think that it would be a short-lived experiment and
> we would be back to CTR pretty quickly.
>
> Is it possible to vote -0.5 instead of -1?  I don't think it is.
>
> Thanks,
> Shawn
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
>

> On Tue, Nov 26, 2019 at 12:34 AM David Smiley <[EMAIL PROTECTED]>
> wrote:
>
>> Last Wednesday at a Solr committers meeting, there was general
>> agreement...
>>
>> I'd prefer we have one "Commit Policy" document for Lucene/Solr and only
>> call out Solr specifics when applicable.  This is easier to maintain and is
>> in line with the joint-ness of Lucene TLP.  So I think it should move to
>> the Lucene cwiki.  Granted there is a possibility this kind of content
>> might move into our source control somewhere but that possibility is a
>> subject for another day.
>>
>
> -1 ... you even went so far as to discourage lucene committers from
> attending that meeting, and now its turned around as if its consensus
> everywhere and should be applied to lucene too?
>

> This particular committer's meeting has a particular subject/theme.  So
> as Erick indicated, while all committers are invited, I think if you're
> not interested in the subject then I'm sure you can find a better use of
> your time.

> I don't think changing things to review-then-commit will help.
>

> Hello Rob,
>
> On Sat, Nov 30, 2019 at 9:39 AM Robert Muir <[EMAIL PROTECTED]> wrote:
>
>> On Tue, Nov 26, 2019 at 12:34 AM David Smiley <[EMAIL PROTECTED]>
>> wrote:
>>
>>> Last Wednesday at a Solr committers meeting, there was general
>>> agreement...
>>>
>>> I'd prefer we have one "Commit Policy" document for Lucene/Solr and only
>>> call out Solr specifics when applicable.  This is easier to maintain and is
>>> in line with the joint-ness of Lucene TLP.  So I think it should move to
>>> the Lucene cwiki.  Granted there is a possibility this kind of content
>>> might move into our source control somewhere but that possibility is a
>>> subject for another day.
>>>
>>
>> -1 ... you even went so far as to discourage lucene committers from
>> attending that meeting, and now its turned around as if its consensus
>> everywhere and should be applied to lucene too?
>>
>
> I'm looking back over what I wrote.  I suppose it's this self-quote that
> bothered you?:
>
>> This particular committer's meeting has a particular subject/theme.  So
>> as Erick indicated, while all committers are invited, I think if you're
>> not interested in the subject then I'm sure you can find a better use of
>> your time.
>
> Personally I think what I stated there is reasonable.  More importantly,
> both me and Erick also expressly declared all committers are welcome.  If I
> actually meant otherwise then I wouldn't have said so.
>
>
>> I don't think changing things to review-then-commit will help.
>>
>
> I don't think *anyone* wants ASF's RTC due to the "consensus approval"
> requirement with three +1's --
> http://www.apache.org/foundation/glossary.html#ReviewThenCommit  *My
> proposal says this up front clearly* (I think).  Did you read it?  I
> don't think Shawn read it either -- I'm not calling for an official change
> to RTC.  Here's the link again:
> https://cwiki.apache.org/confluence/display/LUCENE/Commit+Policy+-+DRAFT
> I can copy-paste it here if you like which would also serve to snapshot
> exactly what I'm talking about.
>
> I observe that we seem to mostly follow this guideline document already,
> particularly what I observe in Lucene (less so Solr).  No not all the
> details and all the particulars I wrote on what constitutes "minor", but
> the gist of the review/approval.  We don't just commit at will today; we
> wait for reviews and get them.  I'd like what we do today (in Lucene)
> written down more clearly for everyone's benefit -- new/interested people
> and ourselves.  This document is my attempt to do that plus raising the
> review/approval guideline norms *just a little bit* (IMO).  For Solr it's
> more than a little bit, granted.  Then I can point others at this, and if I
> see behavior that got no review for something where it was warranted
> (according to documented guidelines), I can reference this.
>
> Lets say we accept these new guidelines.  After six months, we can
> change/loosen our practices and edit this document accordingly.  It's just

> 2. des. 2019 kl. 09:00 skrev Robert Muir <[EMAIL PROTECTED]>:
>
> 
> yes David: I read the document, I'm against both the contents of it, and how it came about.  For example there is no "silence is consensus" which is really important to me. If you work full-time on this shit for some large company X, this is probably of no concern to you.
>
> But it matters to me. For example, I opened some patches to improve solr's security because its currently an RCE-fest. I'm gonna wait a couple days, if nobody says anything about these patches I opened for Solr, I'm gonna fucking commit them: someone needs to address this stuff. Why should I wait weeks/months for some explicit review? There is repeated RCE happening, how the hell could I make anything worse?
>
> Please don't add unnecessary things to this document like "Linear git history" which have nothing to do with code reviews. Its already controversial as its trying to invent its own form of "RTC", you aren't helping your cause.
>
>
>> On Mon, Dec 2, 2019 at 12:35 AM David Smiley <[EMAIL PROTECTED]> wrote:
>> Hello Rob,
>>
>> On Sat, Nov 30, 2019 at 9:39 AM Robert Muir <[EMAIL PROTECTED]> wrote:
>>>> On Tue, Nov 26, 2019 at 12:34 AM David Smiley <[EMAIL PROTECTED]> wrote:
>>>>> Last Wednesday at a Solr committers meeting, there was general agreement...
>>>>>
>>>>> I'd prefer we have one "Commit Policy" document for Lucene/Solr and only call out Solr specifics when applicable.  This is easier to maintain and is in line with the joint-ness of Lucene TLP.  So I think it should move to the Lucene cwiki.  Granted there is a possibility this kind of content might move into our source control somewhere but that possibility is a subject for another day.
>>>>
>>>> -1 ... you even went so far as to discourage lucene committers from attending that meeting, and now its turned around as if its consensus everywhere and should be applied to lucene too?
>>>
>>> I'm looking back over what I wrote.  I suppose it's this self-quote that bothered you?:
>>> This particular committer's meeting has a particular subject/theme.  So as Erick indicated, while all committers are invited, I think if you're not interested in the subject then I'm sure you can find a better use of your time.
>> Personally I think what I stated there is reasonable.  More importantly, both me and Erick also expressly declared all committers are welcome.  If I actually meant otherwise then I wouldn't have said so.
>>  
>>> I don't think changing things to review-then-commit will help.
>>
>> I don't think anyone wants ASF's RTC due to the "consensus approval" requirement with three +1's -- http://www.apache.org/foundation/glossary.html#ReviewThenCommit  My proposal says this up front clearly (I think).  Did you read it?  I don't think Shawn read it either -- I'm not calling for an official change to RTC.  Here's the link again: https://cwiki.apache.org/confluence/display/LUCENE/Commit+Policy+-+DRAFT
>> I can copy-paste it here if you like which would also serve to snapshot exactly what I'm talking about.
>>
>> I observe that we seem to mostly follow this guideline document already, particularly what I observe in Lucene (less so Solr).  No not all the details and all the particulars I wrote on what constitutes "minor", but the gist of the review/approval.  We don't just commit at will today; we wait for reviews and get them.  I'd like what we do today (in Lucene) written down more clearly for everyone's benefit -- new/interested people and ourselves.  This document is my attempt to do that plus raising the review/approval guideline norms just a little bit (IMO).  For Solr it's more than a little bit, granted.  Then I can point others at this, and if I see behavior that got no review for something where it was warranted (according to documented guidelines), I can reference this.

>
>
> There have been examples of some rather sloppy and rapid commits in Solr
> of non-trivial core code that turned out to cause bugs, corruption and
> perhaps even security issues.
>
>

> I think the distanse is not necessarily as large as it seems. Nobody wants
> to get rid of lazy consensus, but rather put down in writing when we
> recommend to wait for explicit review.
>

>
>
> On Mon, Dec 2, 2019 at 3:49 AM Jan Høydahl <[EMAIL PROTECTED]> wrote:
>
>> I think the distanse is not necessarily as large as it seems. Nobody
>> wants to get rid of lazy consensus, but rather put down in writing when we
>> recommend to wait for explicit review.
>>
>
> Then change the document's name to be Recommendation instead of Policy!
>

> > For example, I opened some patches to improve solr's security because
> its currently an RCE-fest. I'm gonna wait a couple days, if nobody says
> anything about these patches I opened for Solr, I'm gonna fucking commit
> them: someone needs to address this stuff. Why should I wait weeks/months
> for some explicit review? There is repeated RCE happening, how the hell
> could I make anything worse?
>
> +1 Robert, totally agree. RCE etc should be absolutely top priority.
> Thanks a ton for tackling this. Breaking functionality (not deliberately of
> course) is better than having RCEs in a release. IOW, it can't get worse.
>
> On Mon, 2 Dec, 2019, 3:03 PM Robert Muir, <[EMAIL PROTECTED]> wrote:
>
>>
>>
>> On Mon, Dec 2, 2019 at 3:49 AM Jan Høydahl <[EMAIL PROTECTED]> wrote:
>>
>>> I think the distanse is not necessarily as large as it seems. Nobody
>>> wants to get rid of lazy consensus, but rather put down in writing when we
>>> recommend to wait for explicit review.
>>>
>>
>> Then change the document's name to be Recommendation instead of Policy!
>>
>

> Last Wednesday at a Solr committers meeting, there was general agreement
> in attendance to raise the bar for commit permission to require another's
> consent, which might not have entailed a review of the code.  I volunteered
> to draft a proposal.  Other things distracted me but I'm finally thinking
> of this task now.  *This email is NOT the proposal*.
>
> I was about to write something from scratch when I discovered we already
> have some internal documentation on a commit policy that is both reasonably
> well written/composed and the actual policy/information is pretty good --
> kudos to the mystery author!
>
> https://cwiki.apache.org/confluence/display/SOLR/CommitPolicy
>
> I'd prefer we have one "Commit Policy" document for Lucene/Solr and only
> call out Solr specifics when applicable.  This is easier to maintain and is
> in line with the joint-ness of Lucene TLP.  So I think it should move to
> the Lucene cwiki.  Granted there is a possibility this kind of content
> might move into our source control somewhere but that possibility is a
> subject for another day.
>
> I plan to copy this to Lucene, mark as PROPOSAL and then make some large
> edits.  The diff will probably be kinda unrecognizable despite it being in
> nice shape now.  A "Commit Policy" is more broad that a "Code Review
> Policy"; it could cover a variety of things.  For example when to commit
> without even filing a JIRA issue, which I think is worth mentioning.  It
> should probably also cover Git considerations like merge vs rebase, and
> multiple commits vs squashing.  Maybe we should also cover when to bother
> adding to CHANGES.txt and "via"?  Probably commit message requirements.
> Snowballing scope :-). Probably not JIRA metadata as it's not part of the
> commit to be part of a commit policy, but _somewhere_ that's needed.  I'm
> not sure I want to  sign up for all that now but at least for the code
> review subject.
>
> ~ David Smiley
> Apache Lucene/Solr Search Developer
> http://www.linkedin.com/in/davidwsmiley
>

> Why should I wait weeks/months for some explicit review
>

>
>    1. It would be nice if JIRA had a (*optional!*) PATCH_REVIEW status
>    (down stream from PATCH_AVAILABLE and it's infrastructure, but not
>    requiring PATCH_AVAILABLE as a prior step) that could be hooked up to send
>    a mail with an easily filterable subject line to individuals that have
>    (somehow) nominated themselves as interested in the associated Jira
>    Component
>
> I was thinking of this the other day.  Hmm. Lets start a new thread on

> > there is no "silence is consensus"
>
> Good point, maybe we should include something about this too. While
> hopefully this doesn’t happen much, it doesn’t make sense to stall work for
> weeks after putting up a final patch. Again, I think the doc’s intention is
> to be a guideline, but if there are exceptions, so be it.
>

> Please don't add unnecessary things to this document like "Linear git
> history" which have nothing to do with code reviews.
>

> Thanks David, and every one else here.
>
> Let's not call this policy, as 1. there are folks other than me who think
> that's too strong a word for what we are trying to do here, 2. this has
> "rules" that we will have to ignore based on the circumstance. these rules
> should instead be recommendations, and the policy be either what Robert or
> Tomas suggested i.e. Recommendation/Guidelines.
>
> I feel that the document might be read in a different context by people
> who weren't in the meeting. As far as I understand, the intention here is
> to have better quality commits, with involvement of more than one person,
> especially when the changes are large and affect the core/API.
>
> There are points around adding tests, which also directly are about just
> improving code quality, something that has led to the failing tests and
> other issues that have been spoken about in the recent past.
>
> +1 on adding something explicitly about "silence is consensus".
> There are certain parts of the code that aren't worked by more than a
> couple of people and in that case, these points will have to be just
> interpreted based on the situation.
>
> Overall, I really think it will not only be awesome  but is critical to

>
> Rob wrote:
>
>> Why should I wait weeks/months for some explicit review
>>
> Ask for a review, which as this document says is really just a LGTM
> threshold of approval, not even a real code review.  Given your reputation
> of writing quality code, this isn't going to be an issue for you.  If it's
> taking multiple weeks for anyone then we have a problem to fix -- and at
> present we do in Solr.  Explicitly encouraging mere approvals (as the
> document says) should help a little.  Establishing that we want this
> standard of conduct as this document says (even if not mandatory) will also
> help -- "you scratch my back, I scratch yours".  But I think we should do
> even more...
>
>

>
>
> On Mon, Dec 2, 2019 at 3:33 PM David Smiley <[EMAIL PROTECTED]>
> wrote:
>
>>
>> Rob wrote:
>>
>>> Why should I wait weeks/months for some explicit review
>>>
>> Ask for a review, which as this document says is really just a LGTM
>> threshold of approval, not even a real code review.  Given your reputation
>> of writing quality code, this isn't going to be an issue for you.  If it's
>> taking multiple weeks for anyone then we have a problem to fix -- and at
>> present we do in Solr.  Explicitly encouraging mere approvals (as the
>> document says) should help a little.  Establishing that we want this
>> standard of conduct as this document says (even if not mandatory) will also
>> help -- "you scratch my back, I scratch yours".  But I think we should do
>> even more...
>>
>>
>  Why should I ask for your review? It's not even your code thats running
> anymore, its the hackers code :)
>
>
>

> > Why should I ask for your review? It's not even your code thats running
> anymore, its the hackers code :)
>
> Haha! +1 on moving ahead with RCEs and other security issues without
> needing to wait for reviews. Waiting for reviews (esp. if no one has enough
> bandwidth for quick reviews) for such crucial issues can risk dragging
> those issues on and on needlessly. Reviews can happen after commit too, if
> people have the time.
>
> On Tue, 3 Dec, 2019, 6:51 AM Robert Muir, <[EMAIL PROTECTED]> wrote:
>
>>
>>
>> On Mon, Dec 2, 2019 at 3:33 PM David Smiley <[EMAIL PROTECTED]>
>> wrote:
>>
>>>
>>> Rob wrote:
>>>
>>>> Why should I wait weeks/months for some explicit review
>>>>
>>> Ask for a review, which as this document says is really just a LGTM
>>> threshold of approval, not even a real code review.  Given your reputation
>>> of writing quality code, this isn't going to be an issue for you.  If it's
>>> taking multiple weeks for anyone then we have a problem to fix -- and at
>>> present we do in Solr.  Explicitly encouraging mere approvals (as the
>>> document says) should help a little.  Establishing that we want this
>>> standard of conduct as this document says (even if not mandatory) will also
>>> help -- "you scratch my back, I scratch yours".  But I think we should do
>>> even more...
>>>
>>>
>>  Why should I ask for your review? It's not even your code thats running
>> anymore, its the hackers code :)
>>
>>
>>

> https://cwiki.apache.org/confluence/display/LUCENE/Commit+Policy+-+DRAFT
>
> Updated:
> * Suggested new title
> * Emphasizing "Guidelines" instead of policy
> * Defines lazy-consensus
> * Added [PENDING DISCUSSION] to other topics for now
>
> Question:
> * Are we agreeable to my definition of "minor"?
> * Do we agree we don't even need a JIRA issue for "minor" things?
> * Do we agree we don't even need a CHANGES.txt entry for "minor" things?
> Of course it's ultimately up to the committer's discretion but I ask as a
> general guideline.  If we can imagine some counter examples then they might
> be good candidates to add to the doc.
>
> ~ David Smiley
> Apache Lucene/Solr Search Developer
> http://www.linkedin.com/in/davidwsmiley

> 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

> As more of a practioner/user of Solr, I would second a desire to have more
> review involved in what gets added. I am sometimes surprised at features
> that have gotten added with minimal review that feel fairly experimental or
> impact stability. I don't think it's anything against the people working on
> the features, as a sometimes contributor, I too have not fully thought out
> all the implications, big and small, of my desired changes. I have been
> rather impressed how much my contribution has improved when a
> committer (namely Mr. Smiley, who is an incredible human being) has helped
> review & shephard the change.
>

>
> On Tue, Dec 3, 2019 at 3:02 PM Doug Turnbull <
> [EMAIL PROTECTED]> wrote:
>
>> As more of a practioner/user of Solr, I would second a desire to have
>> more review involved in what gets added. I am sometimes surprised at
>> features that have gotten added with minimal review that feel fairly
>> experimental or impact stability. I don't think it's anything against the
>> people working on the features, as a sometimes contributor, I too have not
>> fully thought out all the implications, big and small, of my desired
>> changes. I have been rather impressed how much my contribution has improved
>> when a committer (namely Mr. Smiley, who is an incredible human being) has
>> helped review & shephard the change.
>>
>
> 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)
>
> Solr really must *remove features* from time to time, and then refactor
> the code to remove any hacks those features brought in, like a normal
> software project.
>
> So instead of adding a bunch of policy, it might be a better idea to
> directly attack the problem of piling up features, maybe declare a
> moratorium on new features for a while, remove some outdated ones, and do
> some long overdue cleanup.
>

> 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.
>

> 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.
>

>
> 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 <[EMAIL PROTECTED]> 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?
>

> 4. des. 2019 kl. 22:49 skrev Noble Paul <[EMAIL PROTECTED]>:
>
>> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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?

> 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 <[EMAIL PROTECTED]>:
> >
> >> 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 <[EMAIL PROTECTED]>
> 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 <[EMAIL PROTECTED]> 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.

> 5. des. 2019 kl. 14:22 skrev Robert Muir <[EMAIL PROTECTED]>:
>
> 
> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]>:
>> >
>> >> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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.

> 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 <[EMAIL PROTECTED]>:
>
> 
> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]>:
>> >
>> >> 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 <[EMAIL PROTECTED]>
>> 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 <[EMAIL PROTECTED]> 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

>
> And a link to guidelines on what goes where....
>
> On Thu, Dec 5, 2019 at 10:49 AM Jan Høydahl <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]>:
>>
>> 
>> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]>:
>>> >
>>> >> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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.