Skip to content

Latest commit

 

History

History
548 lines (394 loc) · 27.9 KB

contributing.md

File metadata and controls

548 lines (394 loc) · 27.9 KB
Raw

Contributing to Ignition

Thank you for your interest in contributing to Ignition!

The following is a set of guidelines for contributing to Ignition and its components, which are hosted in the Ignition Organization on GitHub. These are mostly guidelines, not rules. Use your best judgment, and feel free to propose changes to this document in a pull request.

Table of Contents

Code of Conduct

Project Design

How to Contribute

Writing Tests

Styleguides

Appendix

Code of Conduct

This project and everyone participating in it is governed by the Ignition Code of Conduct. By participating, you are expected to uphold this code. Please report unacceptable behavior at https://ignitionrobotics.org/support.

Project Design

Repository List

The following is a list of the major Ignition repositories. A complete list can be found at https://github.com/ignitionrobotics/.

  • ign-cmake: CMake modules used to configure and build Ignition libraries.
  • ign-common: Set of general purpose components and utilities, such as 3D mesh processing, console logging, and signal handling.
  • ign-fuel-tools: Interact with Ignition Fuel servers.
  • ign-gazebo: A high-fidelity 3D rigid-body dynamic simulator.
  • ign-gui: QT-based library to configure and manage graphical applications.
  • ign-launch: Launch executables and plugins.
  • ign-math: A math library targeted toward robotic applications.
  • ign-msgs: Protobuf messages and utilities for simulation and robotics.
  • ign-physics: Plugin based library for physics simulation.
  • ign-plugin: Library for registering, loading, and managing plugin libraries.
  • ign-rendering: Library that supports rendering through different engines, such as OGRE and Optix.
  • ign-sensors: A set of models that generate realistic sensor data.
  • ign-tools: Provides the ign command line interface that can be configured and used by multiple libraries.
  • ign-transport: High performance inter- and intra-process communication based on ZeroMQ and Protobuf.
  • sdformat: World description format.

How to Contribute

Reporting Bugs

Before Submitting a Bug Report

  1. Check the questions and answers forum. Your issue may have already been resolved.
  2. Determine the repository which should receive the problem.
  3. Search the repository's issues to see if the same or similar problem has been opened. If it has and the issue is still open, then add a comment to the existing issue. Otherwise, create a new issue.

How to Submit a Good Bug Report

Create an issue on the repository that is related to your bug, explain the problem, and include additional details to help maintainers reproduce the problem. Refer to the Short, Self Contained, Correct (Compilable), Example Guide as well as the following tips:

  • Use a clear and descriptive title for the issue to identify the problem.
  • Describe the exact steps which reproduce the problem in as many details as possible. When listing steps, don't just say what you did, but explain how you did it.
  • Provide specific examples to demonstrate the steps. Include links to files or projects, or copy/pasteable snippets, which you use in those examples.
  • Describe the behavior you observed after following the steps and point out what exactly is the problem with that behavior.
  • Explain which behavior you expected to see instead and why.
  • Include screenshots and animated GIFs which show you following the described steps and clearly demonstrate the problem. See Creating GIFs for GIF creation utilities.
  • If the problem wasn't triggered by a specific action, describe what you were doing before the problem happened and share more information using the guidelines below.

Provide more context by answering these questions:

  • Did the problem start happening recently (e.g. after updating to a new version) or was this always a problem?
  • If the problem started happening recently, can you reproduce the problem in an older version? What's the most recent version in which the problem doesn't happen?
  • Can you reliably reproduce the issue? If not, provide details about how often the problem happens and under which conditions it normally happens.
  • If the problem is related to working with files, does the problem happen for all files and projects or only some? Does the problem happen only when working with local or remote files, with files of a specific type (e.g. only Collada or SDF), or with large files or files with very long lines? Is there anything else special about the files you are using?

Include details about your configuration and environment:

  • Which version of Ignition are you using??
  • What's the name and version of the OS you're using?
  • Are you running Ignition in a virtual machine? If so, which VM software are you using and which operating systems and versions are used for the host and the guest?

Suggesting Enhancements

This section guides you through submitting an enhancement suggestion, including completely new features and minor improvements to existing functionality. Following these guidelines helps maintainers and the community understand your suggestion and find related suggestions.

Before creating enhancement suggestions, please check this list as you might find out that you don't need to create one. When you are creating an enhancement suggestion, please include as many details as possible. When filling in the issue form for an enhancement suggestion, include the steps that you imagine you would take if the feature you're requesting existed.

Before Submitting An Enhancement Suggestion

  • Check if you're using the latest software version. A more recent version may contain your desired feature.
  • Check if there's already a library which provides that enhancement.
  • Determine which repository the enhancement should be suggested in.
  • Perform a cursory search to see if the enhancement has already been suggested. If it has, add a comment to the existing issue instead of opening a new one.
  • Ask on the community forum about your feature. Someone else might already have started, and you might be able to help.

How Do I Submit A (Good) Enhancement Suggestion?

Enhancement suggestions are tracked as GitHub issues. After you've determined which repository your enhancement suggestion is related to, create an issue on that repository and provide the following information:

  • Use a clear and descriptive title for the issue to identify the suggestion.
  • Provide a step-by-step description of the suggested enhancement in as many details as possible.
  • Provide specific examples to demonstrate the steps. Include copy/pasteable snippets which you use in those examples, as Markdown code blocks.
  • Describe the current behavior and explain which behavior you expected to see instead and why.
  • Include screenshots and animated GIFs which show you following the described steps and clearly demonstrate the problem. See Creating GIFs for GIF creation utilities.
  • Explain why this enhancement would be useful to most users and isn't something that can or should be implemented as a separate application.
  • Specify which version of Ignition you're using.
  • Specify the name and version of the OS you're using.

Contributing Code

We follow a development process designed to reduce errors, encourage collaboration, and make high quality code. Review the following to get aquainted with this development process.

  1. Read the Reporting Bugs and Suggesting Enhancements sections first.

  2. Fork the Ignition library you want to contribute to. This will create your own personal copy of the library. All of your development should take place in your fork.

    • An important thing to do is create a remote pointing to the upstream remote repository. This way, you can always check for modifications on the original repository and always keep your fork repository up to date.

  3. Choose a base branch. If your changes will break API or ABI, then base your new branch off of master. If your changes don't break API/ABI and you would like them to be released to an existing release with major version N, then use branch ign-<library>N as the base.

  4. Work out of a branch Always work out of a new branch, one that is not a release / master branch. This is a good habit to get in, and will make your life easier.

  5. Write your code. This is the fun part, but is good to remember:

    • Always sign your commits (See the bullet about Developer Certificate of Origin in the Process section below)
    • Look at the existing code and try to maintain the existing style and pattern as much as possible
    • Always keep your branch updated with the original repository

  6. Write tests. In most cases, a pull request will only be accepted if it has tests. See the Writing Tests section below for more information.

  7. Resolve compiler warnings. Code must have zero compile warnings, or at least make sure your pull request is not adding new warnings.

  8. Follow the style guide.

    Static code checking analyzes your code for bugs, such as potential memory leaks, and style. Most Ignition libraries use the cppcheck static code checker, and a modified version cpplint. Ubuntu users can install via:

     sudo apt-get install cppcheck

    To check your code, run the following script from the build folder of the project that you're working on. If you're working on ignition-math, for instance, the path for the folder should be something similar to ~/citadel_ws/build/ignition-math6. The path example is assuming you followed our installation instructions using colcon.

    Then, run the script inside this folder:

     make codecheck

    This may take a few minutes to run. Fix all errors and warnings until the output looks like:

     Built target codecheck

    The tool does not catch all style errors. See the code style section below for more information.

  9. (optional) Use clang-tidy for additional checks.

    clang-tidy should return no errors when run against the code base.

    Ubuntu users can install via:

     sudo apt-get install clang-tidy-6.0 libclang-6.0-dev python-yaml

    clang-tidy can then be executed by running from the source dir:

     ./tools/clang_tidy.sh

    Address issues that are found.

    If you are feeling adventurous, you can experiment with adding additional checks to the .clang-tidy file by referencing the full list of options in the clang-tidy documentation

  10. Tests must pass. You can check by running make test in your build directory. Running tests may take a bit of time, be patient.

  11. Write documentation. Document all your code. Every class, function, member variable must have doxygen comments. All code in source files must have documentation that describes the functionality. This will help reviewers and future developers.

  12. Review your code. Before submitting your code through a pull request, take some time to review everything line-by-line. The review process will go much faster if you make sure everything is perfect before other people look at your code. There is a bit of the human-condition involved here. Folks are less likely to spend time reviewing your code if it's sloppy.

  13. Make small pull requests

    A large pull request is hard to review, and will take a long time. It is worth your time to split a large pull request into multiple smaller pull requests. For reference, here are a few examples:

  14. Submit a pull request to the Ignition library through GitHub when you're ready.

  15. Check Continuous integration

    The moment you make a pull request, a few jobs in our continuous integration server will be started. These jobs will build your branch on Linux, Mac and Windows, run all tests and check for warnings.

    Your pull request will be updated with the status of these builds. Take some time to check these builds and see if you've introduced test failures, warnings or broke some build. If you did and know how to fix it, do so. If you don't know, speak up and someone may try to help you.

  16. Respond to reviewers. At least two other people have to approve your pull request before it can be merged. Please be responsive to any questions and comments.

  17. Done, phew. Once you have met all the requirements, you're code will be merged. Thanks for improving Ignition!

Tracking Progress

Ignition development progress is tracked publicly using a GitHub project board. Using project boards ensures the community has visibility to what’s coming up, external contributors can understand where they can help, and the reasoning behind development decisions are visible to everyone.

Contributors should look at the "Core development" board, though it's possible we may have other boards in our organization at various times.

Repositories

The following repositories from Ignition are tracked:

New issues and pull requests, and issue and pull request statuses, from across the tracked repositories are all automatically synced with the same board.

Unfortunately, GitHub boards' cross-org support is poor. The following repos can't be handled automatically and have to be manually tracked on the board along with the ones listed above:

Board Columns

The project board columns are defined below:

  • Inbox: New issues and pull requests
  • To do: Issues ready to be worked on.
  • In progress: Issues being worked on / pull requests being iterated on.
  • In review: Pull requests under review / issues requesting feedback.
  • Done: Closed issues / pull requests (cleared from time to time).

If a ticket is not on the board, it means that the core team is not planning to work on it, but the community is welcome to do so.

Process

All Ignition team members actively:

  • Watch all Ignition repositories to receive email notifications of new issues / pull requests

  • Provide feedback to issues as soon as possible

  • Review pull requests as soon as possible

    • Team members can review pull requests already under review or approved
    • Team members can provide some feedback without doing a full review

During weekly simulation meetings, the team:

  • Quickly triages the inbox, if there’s anything left in it

    • If we won’t work on an issue in the near future, remove it from the board.
    • If we’re going to work on an issue, move it to To Do, no assignment necessary initially.
    • If we won’t review a pull request in the near future, close it.

  • Discusses In Review column and additional tickets that people want to bring up

  • Discusses the Done column and archives all tickets

When opening a pull-request:

  • Labels according to the targeted collection(s) (blueprint, citadel, etc) will be added automatically

  • Add yourself as the assignee

    • Maintainers of each repo will be also automatically assigned

  • It will be automatically triaged to the “Core development” project board

  • Add reviewers as appropriate

    • If reviewers won’t be able to review soon, they can remove themselves and also let the original author know

Pull requests can be merged when:

  • They have at least 1 approval from a member of the core team

  • There are no unresolved comments

  • CI is passing

  • Developer Certificate of Origin (DCO) check is passing

    • DCO is a declaration of ownership, basically saying that you created the contribution and that it is suitable to be covered under an open source license (not proprietary).
    • All you have to do is end your commit message with Signed-off-by: Your Full Name <your.name@email.com>
    • If your user.name and user.email configurations are set up in git, then you can simply run git commit -s to have your signature automatically appended.

Merging strategy:

  • For internal contributions, give the original author some time to hit the merge button themselves / check directly with them if it’s ok to merge.
  • Default to “squash and merge”
    • Make sure the commit message captures the core ideas of the pull request.
  • “Rebase and merge” when moving files (do a git mv as a separate commit)
  • “Create a merge commit” when porting changes across branches
  • Refrain from force-pushing while the PR is under review (which includes rebasing and squashing)

Porting changes across branches:

  • Pull requests should target the lowest possible supported version where the changes can be added in a backwards-compatible way (no API / ABI / behavior break in released branches).

  • Periodically, a maintainer will forward-port changes to newer release branches all the way up to master.

  • The merge forward can be done with git merge in order to keep the commit history. For example:

      git checkout ign-<library>M
  git pull
  git checkout ign-<library>N
  git pull
  git checkout -b M_to_N_<date> # It's important to do this before `git merge`
  git merge ign-<library>M
  # Fix conflicts
  git commit -sam"Merge M into N"
  # Open pull request

  • In the rare event that a pull request needs to be backported (i.e. from a higher version to a lower version), use git cherry-pick, for example:

      git checkout ign-<library>N
  git pull
  git checkout ign-<library>M
  git pull
  git checkout -b N_to_M_<date>
  git cherry-pick <commits from verrsion N>
  # Fix conflicts
  git commit -sam"Backport from N to M"
  # Open pull request

  • When merging a port pull request, do not squash or rebase, create a merge commit instead.

Writing Tests

Most Ignition libraries use GTest for general testing and QTest for GUI tests. There are a few kinds of tests:

  1. Unit tests: all classes should have corresponding unit tests. These live in the same directory as the source code and are suffixed by _TEST.

  2. Integration tests: tests which verify how many classes are working together go under the tests/integration directory.

  3. Regression tests: tests which fix broken features go under tests/regression and are prefixed by the issue number on librarie's issue tracker.

  4. Performance tests: tests that are designed to check performance characterics, such as CPU or memory usage, go unde tests/performance.

Before creating a new integration or performance test file, check the current test files. If one closely matches the topic of your new code, simply add a new test function to the file. Otherwise, create a new test file, and write your test.

Test Coverage

The goal is to achieve 100% line and branch coverage. However, this is not always possible due to complexity, analysis tools misreporting coverage, and time constraints. Try to write as complete of a test suite as possible, and use the coverage analysis tools as guide. If you have trouble writing a test please ask for help in your pull request.

Ignition CMake provides build target called make coverage that produces a code coverage report. You'll need to have lcov and gcov installed.

  1. In your build folder, compile with -DCMAKE_BUILD_TYPE=Coverage

     cmake -DCMAKE_BUILD_TYPE=Coverage ..\
 make

  2. Run a single test, or all the tests

    ./workspace/build/package_name/UNIT_TestName_TEST   (single test)
 make test                                          (all tests)

  3. Make the coverage report

     make coverage

  4. View the coverage report

     firefox coverage/index.html

Style Guides

You can check code for compliance by running the following command from a build folder:

    make codecheck

In general, we follow Google's style guide and rules set forth by cppcheck. However, we have added some extras listed below.

  1. This pointer

All class attributes and member functions must be accessed using the this-> pointer. Here is an example.

  1. Underscore function parameters

All function parameters must start with an underscore. Here is an example.

  1. Do not cuddle braces

All braces must be on their own line. Here is an example.

  1. Multi-line code blocks

If a block of code spans multiple lines and is part of a flow control statement, such as an if, then it must be wrapped in braces. Here is an example.

  1. ++ operator

This occurs mostly in for loops. Prefix the ++ operator, which is slightly more efficient than postfix in some cases.

  1. PIMPL/Opaque pointer

If you are writing a new class, it must use a private data pointer. Here is an example, and you can read more here.

  1. const functions

Any class function that does not change a member variable should be marked as const. Here is an example.

  1. const parameters

All parameters that are not modified by a function should be marked as const, except for "Plain Old Data" (int, bool, etc). This applies to parameters that are passed by reference, and pointer. Here is an example.

  1. Pointer and reference variables

Place the * and & next to the varaible name, not next to the type. For example: int &variable is good, but int& variable is not. Here is an example.

  1. Camel case

In general, everything should use camel case. Exceptions include SDF element names, and protobuf variable names. Here is an example.

  1. Class function names

Class functions must start with a capital letter, and capitalize every word.

void MyFunction(); : Good

void myFunction(); : Bad

void my_function(); : Bad

  1. Variable names

Variables must start with a lower case letter, and capitalize every word thereafter.

int myVariable; : Good

int myvariable; : Bad

int my_variable; : Bad

  1. No inline comments

// style comments may not be placed on the same line as code.

speed *= 0.44704; // miles per hour to meters per second : Bad

Appendix

Creating GIFs

You can use LICEcap to record GIFs on macOS and Windows, and Silent Cast or Byzanz or Peek on Linux.