Skip to content

Contributing to Autoware

mitsudome-r edited this page Jun 26, 2022 · 2 revisions

This page describes how to contribute to the development of Autoware. It contains both guidelines that should be followed and rules that must be followed.

Communication

Before starting a new contribution, it is important to let the community know what you plan on doing. This serves several purposes.

  1. It lets everyone know that you are going to start work on a feature or bug fix so that no one else does the same and duplicates your effort.
  2. It lets anyone who may already be working on the feature or bug fix know you intend to work on it, so they can tell you and you don't duplicate their effort.
  3. It gives others a chance to provide more information about what the feature or bug fix might need or how it may need to be implemented.

Join the Autoware Discourse and start a new topic introducing yourself and explaining what you want to work on.

If you are so inclined, join the Autoware Slack chat server. This provides a place to have a quick chat to other Autoware users or developers.

Development workflow

The Autoware development workflow is based on these principles:

  • Everyone owns the code. No one person is responsible for maintaining a part of the code, nor does any one person have absolute control over how a part of the code is implemented and managed. These sorts of decisions are arrived at by discussion amongst the community and consensus.
  • Everyone works on everything at some point. If there is a feature or bug fix issue waiting to be developed, take it on. Trying to implement in parts of your code outside of your comfort zone will expand your experience and make you an increasingly valuable contributor, as well as give you the knowledge to help others who run into problems.
  • Be open about problems and be open to suggestions. When you run into a hurdle implementing a new feature or a bug fix, showing your code as it currently is and asking for help will get you advice that will get you over that problem faster than worrying about it yourself. Use Discourse, file an issue, or ask in the Autoware Slack chat. Similarly, when you receive a suggestion on how to better implement something, take that suggestion on board. This is especially important during merge request reviews. Everyone is on the same side and thinking about the quality of the final code.

You will need a GitHub account to contribute to Autoware. To make a new contribution, you will first need to fork the repository into your own account. Following that, you can create a new branch in your forked repository for your new feature or bug fix and make a merge request from there once you have finished developing it. See the sections below for more detail on the steps involved.

If you are not familiar with Git or any of its more complex features such as rebasing commits and merging branches, then read the relevant chapters from the Pro Git SCM book.

Starting a new feature or bug fix

  1. Design your new feature or bug fix. Describe in as much detail as possible how you will implement it, and how it will behave. If it is a bug fix, describe why you believe the bug will be corrected. The more detailed your design, the greater the chance your implementation will be accepted at the merge request review stage. If you can, it often helps to have semi-formal or formal models of your intended design's behaviour and structure.
  2. Communicate your attentions to the Autoware community (see communication, above).
  3. If you have not done so already (such as for a previous feature or bug fix you developed), fork the Autoware repository.
  4. In your new fork, create a new branch by clicking the Branch: master button at the top left, above the list of source files and entering the name for the new branch to create. Important: Make sure your branch begins from the master branch of Autoware, not another feature or bug fix branch, unless someone from the community directs you to do otherwise.
  5. Clone your fork of Autoware to your computer and then check out the branch you have created.
  6. Begin developing your feature or bug fix. Make as many commits as you feel necessary during the development. Important: Be sure to include the Developer Certificate of Origin sign-off line in your commit messages.
    • If you are committing from the command line, pass the -s or --signoff option to git commit to have the sign-off line automatically added to your commit message. Be aware that this implies your agreement with it.
    • You can add the sign-off manually by adding the following line to your commit message: Signed-off-by: Your Name <your.email@example.com>
  7. While working, always build with all tests enabled and run the tests locally before considering your work complete.
  8. Once your feature or bug fix is complete, make a merge request. Consider running style guide checkers checkers and static analysis tools such as cppcheck before making your merge request to catch common errors in advance. These tools will be run automatically on your merge request but fixing errors after making the request will slow down the review process and make others treat your contribution more strictly.

Making a merge request

  1. Before making a merge request, make sure to read the merge request review guidelines to understand what conditions your merge request must meet to be acceptable.
  2. Before making a merge request, run the tests locally to ensure your additions are not going to break anything.
  3. To make a merge request, first push your commits from your local copy to your fork of the Autoware repository on the GitHub servers.
  4. Go to the GitHub page for your Autoware fork and select the branch where your new feature or bug fix has been developed.
  5. Follow the GitHub instructions on creating a new merge request.
  6. When you make the merge request, fill out the merge request template. Provide as much detail about the purpose of the merge request as possible. Include all the design documentation to produced before you started to work on the implementation. If you are implementing a feature or bug fix for which there is an existing issue on the issue tracker, make sure to include the issue number in your merge request comments using text such as "Fixes #42" or "Resolves #42". When giving your merge request a title, do not start the name with tags such as [Feature] or Fix/. Use issue labels instead to indicate what type of merge request it is.
  7. If you know of a contributor who is suitable to review your merge request, request a review from them. This step is optional.

Merge request review guidelines

For a merge request to be acceptable to merge, it must receive at least one favourable review from another member of the Autoware community. It must also not have any negative reviews that have not been resolved through further development or discussion.

Anyone is welcome to perform a merge request review and all comments from all reviews must be considered and answered appropriately to be considered resolved.

A merge request review will look at all aspects of the merge request.

  • Documentation
    • Does the merge request include user guide documentation describing how to use the feature (for new features)?
    • Does the merge request include API documentation for all functions, especially public functions, and for any other public-facing parts of the API such as classes, structures, etc.?
    • Has the design been verified in some way?
  • Source code
    • Does new code and changed code comply with the style guide?
    • Are any new or changed APIs well-designed and do they match the style of the surrounding API?
    • Is the code easy to read and understand, and is it easy to maintain? Fancy coding may impress your friends but it gets merge requests rejected.
    • Is any new source code, including file system layout, well-structured and following best practices?
  • Tests
    • Does the merge request include sufficient tests to achieve maximum practical coverage of the new code and any existing code that was changed? Is this proven by a coverage report from the CI pipeline?
    • Do the tests cover all known edge and corner cases for using the new and changed code?
    • Do all Continuous Integration pipelines pass for all supported platforms? The CI pipelines check not just behaviour but also check things like coding and documentation style and perform static analysis.
    • If appropriate, have simulated or real-world integration tests been performed?
  • Other aspects
    • Each merge request should focus only on one change. That change should be as small as practical to make accurate reviews easier. Other developers are more likely to perform short reviews of simple merge requests that they can understand from reading the changes, than to spend hours reviewing a merge request that changes dozens of files in significant ways.
    • Does the merge request make more changes than necessary to satisfy the feature or bug fix it targets?
    • Does the merge request reference any related issues or merge requests?
    • Can the merge request be merged independent of any other merge requests? This is not an absolute requirement because sometimes multiple merge requests are needed for a complex feature, but in the majority of cases if your merge request requires other merge requests to be merged first, it will be looked at very intensively and may not be accepted. If you are developing a separate feature that requires another feature still in review, consider waiting for that feature to be merged before making your own merge request.
    • Have all comments on the merge request been addressed satisfactorily?
    • Have the commits been signed for the Developer Certificate of Origin?

While performing the review, feel free to suggest changes to the code and documentation. If you are the person requesting the review, enable edits from upstream contributors to speed up the review and improvement process. If you are a particularly dedicated reviewer, fork the repository that the merge request came from and make more substantial changes. Mention the original merge request in your commit message to get your changes added automatically, or if they are changes that require discussion, open a merge request against the original merge request.

In general, we prefer code reviews to be performed by someone who is distant from the new code. This means not getting someone who helped you develop it to do the review, but rather asking for a review from someone else who has contributed to Autoware before. If you do not know who to request a review from, then do not request anyone and the Highfive bot will randomly choose someone to assign from a list of known contributors.

Sometimes the maintainers will ask for reviews from additional contributors. This is not an indication that the other reviews were bad or that the contribution is bad, but is usually a sign that the feature or bug fix is considered particularly critical.

Once a merge request has passed review, it will be merged into the main Autoware repository by one of the maintainers. After this is done you are free to delete the branch you used to develop it in your own fork of the Autoware repository. We recommend you do so to avoid accidentally developing new contributions from that branch rather than from the tip of the Autoware repository.

However, a merge request cannot be merged until at least 48 hours have passed since the merge request was made. This gives others a chance to comment on the merge request before it gets merged. Similarly we prefer that a merge request not be merged until some time has elapsed since the last review was made. This is to give others time to respond to a review with potentially opposing opinions. 24 hours since the last review is the current guideline.

Remember, if you want your merge request merged rapidly then you should communicate your intentions to the community before you begin working. By doing so, you will ensure that everyone interested already knows what is going on when your merge request arrives.

Merging guidelines

If you are a maintainer, you are responsible for merging accepted merge requests into the master branch. In general there is not much involved in this work. When merging a merge request, make sure to follow these guidelines.

  • Read over the merge request's description and discussion, and ensure that there are no outstanding issues with the request.
  • Look over the reviews to ensure they comply with the merge request review guidelines. In particular, keep an eye out for major problems such as unrelated changes included in the merge request, or a missing DCO sign-off line, or incomplete CI runs.
  • If the merge request comes from outside the Autoware organisation, then a maintainer is responsible for running the CI jobs necessary to check if the merge request can be merged. CI must be run on all officially supported platforms.
  • Don't merge merge requests until at least 48 hours have elapsed since the request was made, 24 hours have elapsed since the last review was made, and all comments have been addressed satisfactorily.
  • When performing the merge, squash the commits into as small a number of commits as reasonable. This is done to keep the repository history clean.
  • If the merge request came from a branch on the Autoware repository, rather than from a fork, delete the branch once it has been merged.

Coding style

For contributions to Autoware.AI, choose the style guide appropriate to your contribution. In many cases, there are tools available that can check your code and documentation.

  • For code that is using ROS 1:
  • For code that is using ROS 2, or does not use ROS, follow the appropriate ROS 2 style for the language your code is written in.

Documentation style

All contributions should come with complete documentation. Poor documentation is just as likely to get a merge request rejected as poor code. All documentation must comply with the appropriate style guide.

For user guide documentation, we use ReadTheDocs, which means using Sphinx to write the documentation. Consult the Sphinx documentation for how to write documentation using this tool. In general, follow the Markdown format and provide detailed documentation.

For documenting C++ code, we do not currently have a style guide. Follow the style of the existing code surrounding your contribution.

For documenting Python code, follow PEP 257. Use the pep257 tool to confirm your documentation complies.

Development management

Repository guidelines

The Autoware.AI repository is structured around a single master branch. It follows the OneFlow branching model. However, for merging branches into the master branch we use a rebase-and-squash approach. This allows features to be developed using a large number of commits in a separate branch or fork, while still allowing the final result to be presented in one or a small number of commits in the history of the master branch.

The master branch must always compile without warnings and all tests must always pass. It must always be in a usable state. This implies that any merge request must do the same before it can be merged, and that features must be completed before they are merged.

If a branch is created in the main Autoware repository, then it should be deleted once merged unless there is a good reason for keeping it around, and that reason is discussed and agreed upon by other contributors. We recommend that you treat branches in your own fork in the same way, however this is not required nor enforced in any way.

When releases are made, the head of the release branch is tagged and the branch is deleted. This reduces the number of branches while still providing a known point (the tag) to start version-specific bug-fix branches from. Major releases, minor releases and hot-fix releases are all treated in this way.

Milestones

Autoware.AI uses GitHub milestones to manage the roadmap and the features targeted for each release. Each issue is either targetted at an upcoming milestone, which means it is due to be included in that release, or it is not assigned to any milestone, which means that it is not currently on the roadmap for release.

If you feel that a feature or bug fix should be included in an upcoming release, make a comment on the relevant issue stating which release you think it should be in and why. Stating why you feel this way is important; requests for an issue to be included in a release without any justification are likely to be ignored. If you feel particularly strongly about a feature being released soon and have the development skills, implementing the feature and making a merge request may get it included in the upcoming release, or the one after if the upcoming release is too close.

Issue labels

Issues and merge requests are tagged with labels that describe their current step in the development process, as well as additional aspects such as needing more information from the original reporter. See the labels page for a list of labels used and their meaning.

If you are new to Autoware and looking for a feature or bug-fix to implement, look for those that have the Good first issue label.

If you are filing a new issue or making a merge request, use either the Bug label, for bug reports and bug fixes, or the Enhancement label, for new or improved features and feature requests.