Interested in developing new features, fixing bugs, or contributing code you have laying around? Read on to find out how.
We follow a development process designed to reduce errors, encourage collaboration, and make high quality code. The process may seem rigid and tedious, but every step is worth the effort (especially if you like applications that work).
We will use the Gazebo repository as an example, but the step apply equally well to other repositories maintained by OSRF.
Are you sure?
Run through this mental checklist before getting started.
Has your idea already been done, or maybe someone is already working on it?
This will create your own personal copy of Gazebo. All of your development should take place in your fork.
Work out of a branch:
git checkout -b my_new_branch_name
Always work out of a new branch, never off of master. This is a good habit to get in, and will make your life easier.
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 gazebo release with major version
N, then use branch
gazeboNas the base.
Write your code.
This is the fun part.
A pull request will only be accepted if it has tests. See the
Test coveragesection below for more information.
Code must have zero compile warnings. This currently only applies to Linux.
A tool is provided in Gazebo (and other repositories) to check for correct style. Your code must have no errors after running the following command from the root of the source tree:
The tool does not catch all style errors. See the Style section below for more information.
There must be no failing tests. You can check by running
make testin your build directory.
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.
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 bad.
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:
Submit a pull request from your GitHub fork when you are ready.
At least two other people have to approve your pull request before it can be merged. Please be responsive to any questions and comments.
We have a dashboard that lists all open pull requests. Only pull requests above the green line can be merged, and they must also have at least two approvals. We do this to encourage the review of pull requests. If you submit a pull request, then try to review other open pull requests. This will reduce the time it takes for your code to get accepted, and also gets more eyes on more code.
Once you have met all the requirements, your code will be merged. Thanks for improving Gazebo!
This section is targeted mostly for people who have commit access to the main repositories.
In addition to the general development process, please follow these steps before submitting a pull request. Each step is pass/fail, where the test or check must pass before continuing to the next step.
In general, we follow Google's style guide. However, we add in some extras.
All class attributes and member functions must be accessed using the
this->pointer. Here is an example.
Underscore function parameters
All function parameters must start with an underscore. Here is an example.
Do not cuddle braces
All braces must be on their own line. Here is an example.
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
This occurs mostly in
forloops. Prefix the
++operator, which is slightly more efficient than postfix in some cases.
Any class function that does not change a member variable should be marked as
const. Here is an example.
All parameters that are not modified by a function should be marked as
const. This applies to parameters that are passed by reference, pointer, and value. Here is an example.
Pointer and reference variables
&next to the varaible name, not next to the type. For example:
int &variableis good, but
int& variableis not. Here is an example.
In general, everything should use camel case. Exceptions include SDF element names, and protobuf variable names. Here is an example.
Class function names
Class functions must start with a capital letter, and capitalize every word.
void MyFunction();: Allowed
void myFunction();: Not Allowed
void my_function();: Not Allowed
Variables must start with a lower case letter, and capitalize every word thereafter.
int myVariable;: Allowed
int myvariable;: Not Allowed
int my_variable;: Not Allowed
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: Not Allowed
Accessors must not start with
Member functions granting read access to protected or private data must look like a noun.
public: ::ServerConfig ServerConfig() const;: Allowed
public: ServerConfig GetServerConfig() const;: Not Allowed
- A class name may conflict with an accessor function. For example,
Model(int)would conflict with a
Modelclass. In these cases, try to follow the
Bypattern. For example:
ModelByName(const std::string &_name): Allowed > *
ModelById(const int _id): Allowed
- A template function that returns a data type may use a stand-alone
Get. For example:
- public: template
T Get();` : Allowed
Mutators must start with
Member functions granting write access to protected data must begin with
public: void SetServerConfig(ServerConfig &_config);: Allowed
public: void ServerConfig(::ServerConfig &_config);: Not Allowed
Check to make sure someone else is not currently working on the same feature, before embarking on a project to add something to Gazebo. Simply send a quick email to the Gazebo mailing list expressing your interest and idea. Someone will get back to you shortly about your idea.
All code should have a corresponding unit test. Gazebo uses GTest for unit testing. All regression test should be placed in
Before creating a new regressions 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.
The goal is to achieve 100% line and branch coverage. However, this is not always possible due to complexity issues, 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.
build folder, compile Gazebo and the tests with
cmake -DCMAKE_BUILD_TYPE=Coverage ..\ make make tests
Run a single test, or all the tests
Make the coverage report
View the coverage report
An assertion is a check, which always produce a boolean result, that developers place in the code when want to be sure that check is always true. They are aimed to detect programming errors and should check for impossible situations in the code. If the assertion check failed, the assertion will stop the program immediately.
Object * p = some_crazy_function() GZ_ASSERT(p != NULL, "Object from some_crazy_function should never point to NULL") p->run()
In Gazebo, the GZ_ASSERT macro is designed to handle all our runtime assertions
GZ_ASSERT(<condition to check>,<fail msg>)
Some of the benefits of using the assertions:
While assertions are intended for impossible situations resulting from programming errors, exceptions handle all kinds of expected errors and unusual but logically possible code situations.
Let's review an example: suppose we are writing a math library and created a really fast method to calculate square roots, but it only works for positive numbers. Something declared as:
double sqrt_for_positives(double number)
What could be an assertion and what would be an exception for our revolutionary function?
Exception: If the incoming number is negative (our function only accepts positive numbers), then we will throw an exception. It was an error by the user, but we should consider it a possible scenario since we are offering a public interface.
Assertion: Our square root should never return a negative number. This is not a logical error; it is a completely unexpected error which should be impossible if the function is written correctly.
In order to provide meaningful backtraces when using a debugger, such as GDB, Gazebo should be compiled with debugging support enabled. When using the ubuntu packages, specially the ''-dbg'' package, this support is limited but could be enough in most situations. There are three level of traces which can be obtained:
'''Maximum level of debugging support'''
: This can only be obtained by compiling Gazebo from source and setting the
DEBUG. This will set up debugging symbols with no optimizations. It may be required by developers for problems that are especially difficult to track down.
'''Medium level of debugging support'''
: This can be obtained installing the ''gazebo-dbg'' package (since 1.4 version) or compiling Gazebo from source using the
CMAKE_BUILD_TYPE mode (which is the default if no mode is provided). This will set up ''-O2'' optimization level but provide debugging symbols. This should be the default when firing up gdb to explore errors and submit traces.
'''Minimum level of debugging support'''
: This one is present in package versions previous to 1.4 (no ''-dbg'' package present) or compiling Gazebo from source using the
CMAKE_BUILD_TYPE option. This will set up the maximum level of optimizations and does not provide any debugging symbol information. These traces are particularly difficult to follow.
Code pushed into the Gazebo repository should pass a few simple tests. It is also helpful if patches submitted through GitHub pass these tests. Passing these tests is defined as generating no error or warning messages for each of the following tests.
In your Gazebo build directory run
All the tests should pass. If they do not, you can run and debug the tests individually. For example, to run the transport test from your build directory:
Static code checking analyzes your code for bugs, such as potential memory leaks, and style. The Gazebo static code checker uses cppcheck, and a modified cpplint. You'll need to install cppcheck on your system. Ubuntu users can install via:
sudo apt-get install cppcheck
To check your code, run the following script from the root of the Gazebo sources:
It takes a few minutes to run. Fix all errors and warnings until the output looks like:
Total errors found: 0
CMAKE_BUILD_TYPE=Checkcompiles with no warnings
This test compiles Gazebo with numerous warning flags enabled. The source code for Gazebo should compile cleanly. This does not include code in the
deps directory. As a rule of thumb, start looking for compilation warnings after the proto messages are built which appear as a series of blue text:
Linking CXX executable gazebomsgs_out Running C++ protocol buffer compiler on axis.proto Running C++ protocol buffer compiler on boxgeom.proto Running C++ protocol buffer compiler on camerasensor.proto Running C++ protocol buffer compiler on collision.proto Running C++ protocol buffer compiler on color.proto Running C++ protocol buffer compiler on contact.proto Running C++ protocol buffer compiler on contacts.proto