Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: Re-word a sentence about Jenkins. Mention testing on ARM64

...

  1. Fork the Github repository at http://github.com/apache/kafka if you haven't already 

  2. Clone your fork, create a new branch, push commits to the branch (review the Kafka 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 (doc changes should be submitted along with code change in the same PR).

  4. Run all tests as described in the project's README.

  5. Open a pull request against the trunk branch of apache/kafka. (Only in special cases would the PR be opened against other branches.)

    1. The PR title should usually be of the form KAFKA-xxxx: Title, where KAFKA-xxxx is the relevant JIRA id and Title may be the JIRA's title or a more specific title describing the PR itself. For trivial cases where a JIRA is not required (see JIRA section for more details) MINOR: or HOTFIX: can be used as the PR title prefix.

    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. The easiest is to simply follow GitHub's automatic suggestions. You can 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's ready for review. To do this, click the "Submit Patch" button in JIRA, and then in the resulting dialog, click "Submit Patch".
  8. The project uses Apache Jenkins will not automatic builder for continuous testing on Linux AMD64 and ARM64 build nodes. A CI job will not be started automatically for pull request. A committer needs has to trigger the testing. Feel free to tag a committer and ask for a build trigger.

  9. Once ready, the PR `checks` box will be updated with the test results along with links to the full results on Jenkins (we have separate builds in order to test multiple Java and Scala versions).

  10. Investigate and fix failures caused by the pull the request

    1. Fixes can simply be pushed to the same branch from which you opened your pull request.

    2. Please address feedback via additional commits instead of amending existing commits. This makes it easier for the reviewers to know what has changed since the last review. All commits will be squashed into a single one by the committer via GitHub's squash button or by a script as part of the merge process.

    3. Jenkins will automatically re-test when new commits are pushed.

    4. Despite our efforts, Kafka may have flaky tests at any given point, which may cause a build to fail. You need to ping committers to trigger a new build. 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.

  11. In addition to unit and integration tests, we also have a set of system tests that run on a nightly basis. For large, impactful or risky changes, it is preferable to run the system tests before merging the pull request. Currently that can be done via a manually triggered job in a Jenkins instance provided by Confluent (ask for access in the pull request). In the near future, we will also be able to run the system tests in Travis (progress can be tracked via 

    Jira
    serverASF JIRA
    serverId5aa69414-a9e9-3523-82ec-879b028fb15b
    keyKAFKA-4463
    ). 

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.

  • Please add a comment and "@" the reviewer in the PR if you have addressed reviewers' comments. Even though GitHub sends notifications when new commits are pushed, it is helpful to know that the PR is ready for review once again.

  • Patches can be applied locally following the comments on the JIRA ticket, for example: git pull https://github.com/[contributor-name]/kafka KAFKA-xxxx.

  • 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 by approving it via GitHub's review interface. This indicates the strongest level of technical sign-off on a patch and it means: "I've looked at this thoroughly and take as much ownership as if I wrote the patch myself". If you approve a pull request, you will be expected to help with bugs or follow-up issues on the patch. Consistent, judicious use of pull request approvals is a great way to gain credibility as a reviewer with the broader community. Kafka reviewers will typically include the acronym LGTM in their approval comment. This was the convention used to approve pull requests before the "approve" feature was introduced by GitHub.

  • 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/trunk" 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.

...