Ok, so it's clear what you're proposing then. You want to change the CTR policy. That is indeed quite a big proposal. As I mentioned I'm personally for CTR, but it would be good to hear other peoples thoughts on this.
Joel Bernstein http://joelsolr.blogspot.com/ On Wed, Feb 28, 2018 at 1:30 PM, Tomas Fernandez Lobbe <[email protected]> wrote: > I’m not sure how CTR was put in place either, but it was done 10+ years > ago, when Solr had less than 1/10 of the committers it has now and who > knows how many less production deployments/users. Now Solr is a completely > different project than back then, and what was the correct process then may > not be the correct process now. I’m happy to trade some development speed > for code quality. > > I think just saying “anyone can ask for a review” is not going to be good > enough, this is the case right now, and it rarely happen. > > Tomás > > > On Feb 28, 2018, at 10:17 AM, Joel Bernstein <[email protected]> wrote: > > I agree that code reviews would be a good idea. But to require code > reviews before committing would be a big change in practice for the Solr > committers. I'm not sure how the commit, then review policy was put in > place or what it would mean to change that. Also I would probably > personally vote against a change to the commit and the review policy. > > But, I would be open to encouraging a culture of code review like there is > in Lucene. > > Joel Bernstein > http://joelsolr.blogspot.com/ > > On Wed, Feb 28, 2018 at 12:59 PM, Tomas Fernandez Lobbe <[email protected] > > wrote: > >> In an effort to improve code quality, I’d like to suggest that we start >> requiring code review to non-trivial patches. Not sure if/how other open >> source projects are doing code reviews, but I’ve been using it in internal >> projects for many years and it’s a great way to catch bugs early, some of >> them very difficult to catch in unit tests, like “You are breaking API >> compatibility with this change”, or “you are swallowing >> InterruptedExceptions”, etc. It is also a great way to standardize a bit >> more our code base and to encourage community members to review and learn >> then code. >> In Lucene-land, this is already a common practice but on the Solr side is >> rare to see. It is common on Solr that committer A doesn’t know much about >> component X, so reviewing that may sound useless, but even in that case you >> can provide feedback on the code itself being added (and in the meantime >> learn something about component X). >> >> What do people think about it? >> >> Regarding tools to do it, I’m open to suggestions. I really like Github >> PRs, that now are easy to integrate with Jira and you can create PRs from >> forks or even from two existing branches of the official repo. Also, since >> people is really familiar with them, I expect to encourage reviewers by >> using them. >> >> Tomás >> > > >
