Pull request guidelines#
General pull request workflow#
Autoware uses the fork-and-pull model. For more details about the model, refer to GitHub Docs.
The following is a general example of the pull request workflow based on the fork-and-pull model. Use this workflow as a reference when you contribute to Autoware.
- Create an issue.
- Discuss the approaches to the issue with maintainers.
- Confirm the support guidelines before creating an issue.
- Follow the discussion guidelines when you discuss with other contributors.
- Create a fork repository. (for the first time only)
- Write code in your fork repository according to the approach agreed upon in the issue.
- Write the tests and documentation as appropriate.
- Follow the coding guidelines guidelines when you write code.
- Follow the Testing guidelines guidelines when you write tests.
- Follow the Documentation guidelines guidelines when you write documentation.
- Follow the commit guidelines when you commit your changes.
- Test the code.
- It is recommended that you summarize the test results, because you will need to explain the test results in the later review process.
- If you are not sure what tests should be done, discuss them with maintainers.
- Create a pull request.
- Follow the pull request rules when you create a pull request.
- Wait for the pull request to be reviewed.
- The reviewers will review your code following the review guidelines.
- Not only the reviewers, but also the author is encouraged to understand the review guidelines.
- If CI checks have failed, fix the errors.
- The reviewers will review your code following the review guidelines.
- Address the review comments pointed out by the reviewers.
- If you don't understand the meaning of a review comment, ask the reviewers until you understand it.
- Fixing without understanding the reason is not recommended because the author should be responsible for the final content of their own pull request.
- If you don't agree with a review comment, ask the reviewers for a rational reason.
- The reviewers are obligated to make the author understand the meanings of each comment.
- After you have done with the review comments, re-request a review to the reviewers and back to 6.
- Avoid using force push as much as possible so reviewers only see the differences. More precisely, at least keep a commit history up to the point of review because GitHub Web UI such as the suggested change may require rebase to pass DCO CI.
- If there are no more new review comments, the reviewers will approve the pull request and proceed to 8.
- If you don't understand the meaning of a review comment, ask the reviewers until you understand it.
- Merge the pull request.
- Anyone with write access can merge the pull request if there is no special request from maintainers.
- The author is encouraged to merge the pull request to feel responsible for their own pull request.
- If the author does not have write access, ask the reviewers or maintainers.
- Anyone with write access can merge the pull request if there is no special request from maintainers.
Pull request rules#
Use an appropriate pull request template (required, non-automated)#
Rationale#
- The unified style of descriptions by templates can make reviews efficient.
Example#
There are two types of templates. Select one based on the following condition.
- Standard change:
- Complexity:
- New features or significant updates.
- Requires deeper understanding of the codebase.
- Impact:
- Affects multiple parts of the system.
- Basically includes minor features, bug fixes and performance improvement.
- Needs testing before merging.
- Complexity:
- Small change:
- Complexity:
- Documentation, simple refactoring, or style adjustments.
- Easy to understand and review.
- Impact:
- Minimal effect on the system.
- Quicker merge with less testing needed.
- Complexity:
Steps to use an appropriate pull request template#
- Select the appropriate template, as shown in this video.
- Read the selected template carefully and fill the required content.
- Check the checkboxes during a review.
- There are pre-review checklist and post-review checklist for the author.
Set appropriate reviewers after creating a pull request (required, partially automated)#
Rationale#
- Pull requests must be reviewed by appropriate reviewers to keep the quality of the codebase.
Example#
- For most ROS packages, reviewers will be automatically assigned based on the
maintainer
information inpackage.xml
. - If no reviewer is assigned automatically, assign reviewers manually following the instructions in GitHub Docs.
- You can find the reviewers by seeing the
.github/CODEOWNERS
file of the repository.
- You can find the reviewers by seeing the
- If you are not sure the appropriate reviewers, ask
@autoware-maintainers
. - If you have no rights to assign reviewers, mention reviewers instead.
Apply Conventional Commits to the pull request title (required, automated)#
Rationale#
- Conventional Commits can generate categorized changelogs, for example using git-cliff.
Example#
feat(trajectory_follower): add an awesome feature
Note
You have to start the description part (here add an awesome feature
) with a lowercase.
If your change breaks some interfaces, use the !
(breaking changes) mark as follows:
feat(trajectory_follower)!: remove package
feat(trajectory_follower)!: change parameter names
feat(planning)!: change topic names
feat(autoware_utils)!: change function names
For the repositories that contain code (most repositories), use the definition of conventional-commit-types for the type.
For documentation repositories such as autoware-documentation, use the following definition:
feat
- Add new pages.
- Add contents to the existing pages.
fix
- Fix the contents in the existing pages.
refactor
- Move contents to different pages.
docs
- Update documentation for the documentation repository itself.
build
- Update the settings of the documentation site builder.
!
(breaking changes)- Remove pages.
- Change the URL of pages.
perf
and test
are generally unused.
Other types have the same meaning as the code repositories.
Add the related component names to the scope of Conventional Commits (advisory, non-automated)#
Rationale#
- It helps contributors find pull requests that are relevant to them.
- It makes the changelog clearer.
Example#
For ROS packages, adding the package name or component name is good.
feat(trajectory_follower): add an awesome feature
refactor(planning, control): use common utils
Keep a pull request small (advisory, non-automated)#
Rationale#
- Small pull requests are easy to understand for reviewers.
- Small pull requests are easy to revert for maintainers.
Exception#
It is acceptable if it is agreed with maintainers that there is no other way but to submit a big pull request.
Example#
- Avoid developing two features in one pull request.
- Avoid mixing different types (
feat
,fix
,refactor
, etc.) of changes in the same commit.
Remind reviewers if there is no response for more than a week (advisory, non-automated)#
Rationale#
- It is the author's responsibility to care about their own pull request until it is merged.
Example#
@{some-of-developers} Would it be possible for you to review this PR?
@autoware-maintainers friendly ping.