-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
Pull Request Checklist
- be polite
- encourage contributors (especially new ones)
- always give a lot of contexts
- 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
Depending if this is a breaking change, the target branch needs to be different.
TODO: add more details about this.
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.
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
Use the "Squash and merge" option:
The history looks more clean with this option
- Ensure the milestone is set
- Ensure the labels are set correctly
Helping contributors to have a green CI is important. This section present some how-to to detect common problems
Shippable run this script: bin/utils/ensure-up-to-date
. In the logs it start with:
./bin/utils/ensure-up-to-date
# START SCRIPT: ./bin/utils/ensure-up-to-date
On error case:
UNCOMMITTED CHANGES ERROR
There are uncommitted changes in working tree after execution of 'bin/ensure-up-to-date'
Then you see the result of git diff
(can be use full for debugging) and git status
On error case, the scripts ends with:
Please run 'bin/utils/ensure-up-to-date' locally and commit changes (UNCOMMITTED CHANGES ERROR)
The PR needs to be updated so that the samples corresponds to the changes made to the generator.
If the logs end with:
[ERROR] 5 out of 185 scripts failed.
Grep for ERROR!! FAILED TO RUN
to see which scripts are failing.