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
