This page outlines guidelines for merging Pull Requests.

(warning) NOTE: Due to recently found synchronization issue between GitBox and Github repositories, committers should only use Github repositories for pushing commits.

If you haven't setup Github repository, please refer to the general information page.

Before merging a Pull Request:

  • MUST make sure that all commits reference the appropriate JIRA.
  • SHOULD make sure that all commits have a component tag attached to them.
  • MUST ensure a clean commit history, where each commit is meaningful by itself.
    • MUST squash fixup commits (for example commits that are addressing review comments).
    • Each commit SHOULD address one thing, e.g.:
      • SHOULD NOT fix a bug fix commit and reformatting things at once.
      • Renamings / refactorings / clean ups that do not change semantics/do not add new features.
  • SHOULD rebase the PR and re-run CI tests.
    • It is impractical to require always running CI tests on the latest codebase before merging, because new commits can be merged during the CI execution.
    • SHOULD check the commits merged into the codebase after the CI being triggered.
      • If there is any conflict, or any commit that touches the same components/files that the PR does, it is recommended to at least re-compile the project and re-run the relevant test cases before merging.
  • MUST make sure the latest version of the PR passes the CI tests.
    • Success CI builds from personal Azure also counts. In that case, please link to the build.
    • It’s also acceptable if there is not a complete success build but each stage has succeeded at least once. In that case, please link to all relevant builds.
    • See also Best practice handling test failures.

When merging a Pull Request:

  • MUST NOT use merge commits.
  • SHOULD NOT use the GitHub UI to merge Pull Request.
  • SHOULD add "This closes #PR_ID." to the message of the last commit of the PR.
    • When doing so please make sure to not overwrite existing commit messages!

After merging a Pull Request:

  • SHOULD back port bug-fixes to the 2 last versions (e.g., once 1.7 is released, include them in master, release-1.7, release-1.6).
    • Changes that affect Flink's behavior (in a way that could break setups) SHOULD NOT be back-ported.
  • MUST close the JIRA
    • Add a comment listing the commit hash for each branch respectively.
    • Check the JIRA data
      • Title, components, type and assignee are accurate
      • Title is meaningful
      • FixVersion covers all versions.
        • For release-X.Y branches, pick the next upcoming release version.
        • For master branch, include the next release version (e.g. 1.8.0), unless it was also merged for the first release in the previous series (1.7.0).

Best practice handling test failures:

  • Try reproducing the problem, see if the test failure is deterministic.
    • Try run the test locally.
    • If the problem cannot be reproduced locally, try re-run the CI build.
      • Sometimes the test failures might be deterministic but only in the CI environment.
    • If the test failure is deterministic, it is likely related to the PR changes, and further debugging is needed.
  • For non-deterministic test failures, search on JIRA see if it has been reported.
    • If there’s an existing ticket, report the new instance and link to the build.
    • If there isn’t an existing ticket, create one.
  • For a test instability of very high frequency, that significantly blocks new PRs from merging.
    • Consider upgrading the ticket to Critical or Blocker priority, to receive more attention.
    • Try reaching out to committers with expertise in relevant components to look into the issue.
    • Raise a discussion on temporarily disabling the relevant test cases, in order not to block other PRs.
      • The JIRA ticket must be made a Blocker before disabling the test cases.
      • The test cases must be re-enabled before the JIRA ticket is closed / downgraded.
  • No labels