Skip to content

Pull Request Checklist

Jérémie Bresson edited this page Jul 30, 2018 · 6 revisions

General

  • be polite
  • encourage contributors (especially new ones)
  • always give a lot of contexts

Checks

  • No merge issue
  • Ensure up-to-date scripts
  • Author of the PR is linked to a GitHub account
  • Ensure the Technical Committee is on CC

Target branch

Depending if this is a breaking change, the target branch needs to be different.

TODO: add more details about this.

Review

It should be clear what the PR is changing (link to an issue, explanation, small OpenAPI Spec to reproduce the issue).

Unit tests are always great.

Test locally if needed.

Do not approve changes you do not understand.

If the change is in the common part (is applied for all languages), be extra careful. Ask for a second review.

If necessary, ask the author about his change.

Never merge your own PR

Somebody else needs to approve

Exception to the rule:

  • The PR is only about updating the samples
  • The PR is only about updating the docs
  • The PR is only about adding a unit-test and CI is green

Merge the Pull Request

Use the "Squash and merge" option:

GitHub usage

The history looks more clean with this option

After merge tasks

  • Ensure the milestone is set
  • Ensure the labels are set correctly

Common CI Build issues.

Helping contributors to have a green CI is important. This section present some how-to to detect common problems

Shippable: ensure up to date

TODO: describe it.

Shippable: generate all samples

If the logs end with:

[ERROR] 5 out of 185 scripts failed.

Grep for ERROR!! FAILED TO RUN to see which scripts are failing.