Re: Commit / Code Review Policy

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 li

Re: Commit / Code Review Policy

On Wed, Mar 4, 2020 at 8:36 AM Daniel Ruggeri 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

Re: Commit / Code Review Policy

e 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 mu

Re: Commit / Code Review Policy

l: 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: h

Re: Commit / Code Review Policy

>“Describe your consideration of what goes in solr-core and what goes in >packages or contrib.” +1 for this We should first have a clear definition of what's solr-core and how to create packages On Fri, Dec 6, 2019 at 3:47 AM Gus Heck wrote: > > And a link to guidelines on what goes where

Re: Commit / Code Review Policy

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

Re: Commit / Code Review Policy

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 : > >  > Fine, here's my SIP process. > > You can't add one feature

Re: Commit / Code Review Policy

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 wrote: > Let’s keep this thread about code review guidelines, not about feature > removal, please. And be concrete with proposals for

Re: Commit / Code Review Policy

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

Re: Commit / Code Review Policy

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

Re: Commit / Code Review Policy

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

Re: Commit / Code Review Policy

It's not an either-or. Yes Solr has too many old/deprecated items that aren't getting removed. Yes also many features ought to be in plugins that are opt-in (HDFS). Yes also many of us want to raise the peer-review bar for all the reasons peer reviews bring about better quality software. BTW I

Re: Commit / Code Review Policy

On Tue, Dec 3, 2019 at 3:02 PM Doug Turnbull < dturnb...@opensourceconnections.com> 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

Re: Commit / Code Review Policy

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

Re: Commit / Code Review Policy

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

Re: Commit / Code Review Policy

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

Re: Commit / Code Review Policy

> 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

Re: Commit / Code Review Policy

On Mon, Dec 2, 2019 at 3:33 PM David Smiley 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

Re: Commit / Code Review Policy

The document I wrote is hereby a commit guideline document, not policy. I'll retitle at a later point; it'll change the link. I'll rework a bit so that the tone doesn't sound too absolute. Rob wrote: > Why should I wait weeks/months for some explicit review > Ask for a review, which as this

Re: Commit / Code Review Policy

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,

Re: Commit / Code Review Policy

Some thoughts from reading the doc and this thread 1. This doesn't sound like a huge change, mostly a change in tone since nearly everything in it is a judgement call. 2. I agree that the word policy seems iffy. It has a "consequences for violation" tone to it, and without some sort

Re: Commit / Code Review Policy

Thank you for putting this up, David. While I see Rob’s points, I agree with the proposal in general, and as you and Jan said, this is not too far from what happens today, at least in Lucene-world. > Then change the document's name to be Recommendation instead of Policy! Maybe guidelines? The

Re: Commit / Code Review 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

Re: Commit / Code Review Policy

On Mon, Dec 2, 2019 at 3:49 AM Jan Høydahl 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

Re: Commit / Code Review Policy

On Mon, Dec 2, 2019 at 3:49 AM Jan Høydahl wrote: > > > 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. > > Then the number one thing to do is fix the tests, so that

Re: Commit / Code Review Policy

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. There have been examples of some rather sloppy and rapid commits in Solr of non-trivial core code that turned

Re: Commit / Code Review Policy

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.

Re: Commit / Code Review Policy

Hello Rob, On Sat, Nov 30, 2019 at 9:39 AM Robert Muir wrote: > On Tue, Nov 26, 2019 at 12:34 AM David Smiley > 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

Re: Commit / Code Review Policy

> and now its turned around as if its consensus everywhere David didn't say anything about consensus everywhere. He mentioned pretty clearly that the agreement was only "in attendance", and that this thread was a precursor for putting together a proposal to test the waters further. That all

Re: Commit / Code Review Policy

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.

Re: Commit / Code Review Policy

On Tue, Nov 26, 2019 at 12:34 AM David Smiley 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

Re: Commit / Code Review Policy

I like this new version. This clarifies the review, commit and CHANGES. As a beginner in this process, it helps. I appreciate the idea to have a "risk" section where we could list and say a few words about some risky areas so that the contributor can announce they might be impacted in reviews.

Re: Commit / Code Review Policy

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.

Re: Commit / Code Review Policy

I think the recent replies seem to be in response to the existing document (CommitPolicy), but what David is proposing is to completely rewrite that document into something (his words) "unrecognizable"? So I'll hold off on commenting until we've seen the actual proposal? -Mike On Tue, Nov 26,

Re: Commit / Code Review Policy

I think more is needed going forward. What I would like to see is an explicit "risks" section of the jira that shows the committer has thought about the different risks to the system before committing code that effects the core. The committer should take time to understand what part of the system

Re: Commit / Code Review Policy

+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 wrote: > This document looks reasonable to me and a good description of the way > changes

Re: Commit / Code Review Policy

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

Re: Commit / Code Review Policy

Mystery authors seem to be - Hoss https://web.archive.org/web/20071011105632/http://wiki.apache.org/solr/CommitPolicy - Grant

Commit / Code Review 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