Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

...

  1. Create local branch from master.

    Code Block
    $ git checkout -b BOOKKEEPER-XXXX


  2. Make your modifications, add tests and run the whole test suite.
  3. Commit your changes.
  4. Generate a patch.

    Code Block
    $ git format-patch --no-prefix master


  5. Upload the patch to JIRA.
  6. If the patch is non-trivial, also upload it to review board under bookkeeper-git.
  7. Click on Patch Available in JIRA to signal that the patch is ready to be reviewed.
  8. The jenkin build will build the patch and run various checks like long lines, trailing spaces, findbugs. The pre-commit build report will be posted as a comment to the ticket. Please watch the comment and address the failures observed in pre-commit job and iterator over them.

Git Workflow

Pull Request
  1. Fork the Github repository at http://github.com/apache/bookkeeper if you haven't already.
  2. Clone your fork, create a new branch, push commits to the branch (review the BookKeeper Coding Guidelines, if you haven't already).
  3. Consider whether documentation or tests need to be added or updated as part of the change, and add them as needed.
    1. Doc changes should be submitted along with code change.
    2. New server configuration settings should be added and documented in bookkeeper-server/conf/bk_server.conf, and also documented in doc/bookkeeperConfigParams.textile.
  4. Run all tests with "mvn clean apache-rat:check package findbugs:check" to verify that the code compiles, passes tests and passes findbugs checks.
  5. Open a pull request against the master branch of http://github.com/apache/bookkeeper. (Only in special cases would the PR be opened against other branches.)
    1. The PR title should be of the form BOOKKEEPER-XXXX: Title, where BOOKKEEPER-XXXX is the relevant JIRA and Title may be the JIRA's title or a more specific title describing the PR itself.
    2. If the pull request is still a work in progress, and so is not ready to be merged, but needs to be pushed to Github to facilitate review, then add [WIP] after the JIRA id.
    3. Consider identifying committers or other contributors who have worked on the code being changed. Find the file(s) in GitHub and click "Blame" to see a line-by-line annotation of who changed the code last. You could add @username in the PR description to ping them immediately.
    4. Please state that the contribution is your original work and that you license the work to the project under the project's open source license.
  6. A comment with information about the pull request will be added to the JIRA ticket.
  7. Change the status of the JIRA to "Patch Available" if it is ready for review.
  8. The Jenkins automatic pull request builder will test your changes.
  9. Once ready, the PR `checks` box will be updated with the test results along with a link to the full results on Jenkins.
  10. Investigate and fix failures caused by the pull request.
    1. Fixes can simply be pushed to the same branch from which you opened your pull request.
    • Jenkins will automatically re-test when new commits are pushed, if an existing commit is amended, if the branch is rebased or if the pull request is closed or reopened.
    1. Despite our efforts, BookKeeper may have flaky tests at any given point, which may cause a build to fail. Unfortunately, it's not currently possible to restart the build by issuing a command. A workaround, as stated in `b`, is to close and reopen the PR. If the failure is unrelated to your pull request and you have been able to run the tests locally successfully, please mention it in the pull request.
The Review Process
  • Other reviewers, including committers, may comment on the changes and suggest modifications. Changes can be added by simply pushing more commits to the same branch.
  • Lively, polite, rapid technical debate is encouraged from everyone in the community. The outcome may be a rejection of the entire change.
  • Reviewers can indicate that a change looks suitable for merging with a comment such as: "I think the patch looks good. +1". BookKeeper uses "+1, -1" convention for indicating accepting or rejecting pull request. 
  • Sometimes, other changes will be merged which conflict with your pull request's changes. The PR can't be merged until the conflict is resolved. This can be resolved with "git fetch origin" followed by "git merge origin/master" and resolving the conflicts by hand, then pushing the result to your branch.
  • Try to be responsive to the discussion rather than let days pass between replies.
Closing Your Pull Request / JIRA
  • If a change is accepted, it will be merged and the pull request will automatically be closed, along with the associated JIRA if any
    • Note that in the rare case you are asked to open a pull request against a branch besides master, that you will actually have to close the pull request manually

    • The JIRA will be Assigned to the primary contributor to the change as a way of giving credit. If the JIRA isn't closed and/or Assigned promptly, comment on the JIRA

...

 

 

...

    • .

  • If your pull request is ultimately rejected, please close it promptly

    • ... because committers can't close PRs directly

    • Pull requests will be automatically closed by an automated process at Apache after about a week if a committer has made a comment like "mind closing this PR?" This means that the committer is specifically requesting that it be closed.

  • If a pull request has gotten little or no attention, consider improving the description or the change itself and ping likely reviewers again after a few days. Consider proposing a change that's easier to include, like a smaller and/or less invasive change.

  • If it has been reviewed but not taken up after weeks, after soliciting review from the most relevant reviewers, or, has met with neutral reactions, the outcome may be considered a "soft no". It is helpful to withdraw and close the PR in this case.

  • If a pull request is closed because it is deemed not the right approach to resolve a JIRA, then leave the JIRA open. However if the review makes it clear that the issue identified in the JIRA is not going to be resolved by any pull request (not a problem, won't fix) then also resolve the JIRA.