This document describes the conditions and manner in which a Lucene/Solr committer may commit (git commit, merge and push) new changes to non-feature branches (e.g. to master).  "You" in this document is the committer wishing to make changes.  This policy does not apply to contributors, as they do not have permissions.  Feature branches are WIP that don't directly ship to a release and so there is no policy.

TODO We also discuss some related matters like the commit message and JIRA issues.

Despite the word "Policy" which is suggestive of rules, this document is really a guideline document.  There is a lot of trust between committers; in some cases it may make sense to diverge from these policies.  We try to articulate here when that makes sense.  When in doubt, ask on the dev-list.

Reviews

From an ASF policy perspective, this project follows CTR (Commit-Then-Review) which grants committers permission to commit at will, with the possibility of being retroactively vetoed.  However, in practice code is peer-reviewed before-hand most of the time, and so this project might appear to be the reverse of that.  Strictly speaking, this project does not follow RTC (Review-Then-Commit) which has onerous qualifications we don't wish to adhere to.

By "Review", we mean public approval from someone other than who contributed/wrote the code.  Ideally this approval is explicit like a "+1" or approval using the review system (e.g. GitHub).  If the code is from a non-committer, then the reviewer is "you" here.  Conversely the reviewing person might have no formal relation to the project.  The approval does not have to involve a review of code when there is code.  If you do such an approval, then it's helpful to indicate as-such, e.g. "+1, makes sense but I didn't look at the code".  If you get feedback but no explicit approval, then use your judgment as to wether it was sufficient.  This can be done by announcing your intention to commit in two days if there are no further comments.  Do not count common holidays or weekends in those two days.  Even with a clear approval, you might consider delaying committing if the code was quickly reviewed, thereby giving more time for feedback from others.

Admittedly we have a very low bar for what constitutes a review so as to not block progress.  To make up for this, we have a high bar for becoming a committer and we put a lot of trust on the good-faith judgement of a committer to act in the project's best interest.  Are you modifying code previously unfamiliar to you that you are still unsure of?  Try to get a review from someone familiar with it instead of settling for less.

Don't be shy in asking others for a review.  Earn goodwill and improve the project by reviewing the work of others.

When Reviews Aren't Needed

Reviews are usually but not always required. When changes are sufficiently minor, don't let such processes get in the way of immediate/timely progress.  You probably shouldn't bother waiting for a review for these circumstances:

• Bug fixes (trivial)
• Extensibility improvements that are trivial (e.g. protected/public) on classes that are already marked with lucene.internal or lucene.experimental (or Solr classes/methods deemed similarly even if not annotated).
• Formatting / indentation
• Typo/grammar corrections
• Documentation and code comments
• Error or log message improvement
• Tests, especially additional tests
• dev-tools/

It needs to be minor enough that it's not worth mentioning in CHANGES.txt.  Yet for any guidelines, there are counter-examples.  Are you intending to auto-indent the entire codebase or do any change affecting many files?  Discuss it first.  Are you doing a intra-page re-organization of documentation?  Try to get a review.

Please avoid making lots of little minor commits too.  This is more a matter of taste in reflection of years of our practices.

JIRA has a Priority field with values Minor and Trivial, among others.  We don't differentiate them here; depending on the issue, either values might meet our loose definitions here to skip a review.  

TODO mention not needing a JIRA issue for these scenarios as well?

Tests

There is no real policy on tests, but obviously, we want our code to be tested.  If the proposed changes don't incorporate a test then the issue or GitHub PR (or other review platform) should mention so and why for the benefit of reviewers, unless it's obviously not needed (e.g. already tested via whatever).  Trivial bugs need not.

Bug fix commits should include at least one test that does not pass without the accompanying fix.

Documentation

TODO All new Features should be accompanied by new documentation (at a minimum, mention on the wiki)

TODO triage below old content

Architectural or "Weighty" Changes to the "Guts" of Solr

Changes to Solr's internals should always be tracked as a patch in the issue tracking system under the RTC model. At least one other person knowledgeable in the code that is changing should review the patch before it is committed.

Changes of this scope should almost always include new Unit Tests of whatever internal APIs are changing.

if an architectural or other serious internal change requires changes to existing unit tests, that should be pointed out and the potential ramifications for existing end users should be discussed thoroughly on the Solr developer list.

Non-Backwards Compatible Changes to Functionality or APIs

Changes to Solr's public APIs (either via URL structure, or APIs exposed to Plugin Developers) should always be tracked as a patch in the issue tracking system under the RTC model. Potential ramifications for existing end users should be discussed thoroughly on the Solr user list.

Changes of this scope should result in a Major Version increase (ie: 1.x to 2.0)