You are viewing an old version of this page. View the current version.

Compare with Current View Page History

« Previous Version 2 Next »

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 an explicit "+1".  If the code is from a non-committer, then the reviewer is "you" here.  The reviewing person might have no formal relation to the project.  Sometimes such approval does not 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.  

As previously stated, our guidelines rely on trust 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.

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

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

  • Bug fixes (trivial)
  • Formatting / indentation
  • Typo/grammar corrections and indentation
  • Documentation / code comments
  • Error or log message improvement
  • Tests, especially additional tests

TODO counter-examples of above that ought to have a review

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

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)

  • No labels