The following process describes steps to contribute code changes to the
It follows best practices from industry to ensure a maintainable, high quality code base.
The shown commands assume that
biodynamo/build is the current directory.
If you follow these steps it will make life of the code reviewer a lot easier! Consequently, it will ensure that your code is accepted sooner :)
git clone https://github.com/BioDynaMo/biodynamo.git
git checkout master
git pull origin master
git checkout -b my-feature-branch
You can make intermediate commits without performing all subsequent steps. However, for your final submission these steps are essential.
Also for intermediate commit messages: have a look at how to write good commit messages!
make && make check
Please make sure that there are no compiler warnings
Check if code is sufficiently covered by tests.
make coverage-build # open it in browser - e.g. chromium-browser coverage/coverage/index.html
Check if code changes affected performance
Write documentation and check result in browser
make doc chromium-browser doc/html/index.html
- API documentation has been generated correctly
- it is consistent with code (copy-paste errors)
- it sufficiently describes the functionality
Please pay attention to warnings from doxygen generation. Here an example of an inconsistent documentation:
# make doc ouput: ... kd_tree_node.h:132: warning: argument 'axis' of command @param is not found in the argument list of bdm::spatial_organization::KdTreeNode< T >::GetSAHSplitPoint() kd_tree_node.h:132: warning: argument 'num' of command @param is not found in the argument list of bdm::spatial_organization::KdTreeNode< T >::GetSAHSplitPoint()
The corresponding code snippet shows a mismatch between code and documentation which needs to be fixed.
/// Gets point, which we use for surface area heuristics /// @param axis - on what axis are we separating: x=0,y=1,z=2 /// @param num - what parttion are we on (1;N) /// @return sah rating Point GetSAHSplitPoint();
This command will execute all tests, check code formatting, styleguide rules, build the documentation and coverage report (more info).
False positives from
clang-tidy can be silenced by adding
// NOLINT at the end of the line.
clang-format for a certain part can be done by encapsulating it with the following comments:
// clang-format off code here is not changed by clang-format // clang-format on
If there are no false positives and you are fine with the changes suggested by
make fix-submission. However, failing build, tests, compiler warnings, issues from cpplint and warnings from doxygen must be fixed manually. Also some
clang-tidy issues cannot be resolved automatically. After running
make fix-submission please execute
make check-submission to see if all issues have been resolved.
Please verify that:
- code compiles without warnings
- all tests pass
- all valgrind tests pass
- code complies with our coding styleguide -- no errors from
- documentation is in good order -- see point 10
- code is sufficiently covered by test cases
- performance did not degrade due to the code changes
make check-submission does not report any issues, the final commit can be done.
Have a look at how to write good commit messages!
git add -i git commit
Please create a pull request
Open the Travis-CI build for Linux and OSX and go through the checklist from point 11 for each of them. Unlike compilation and test suite execution, problems caused by formatting, code style and documentation will not fail the build. However, they need to be fixed!
If code changes are necessary, go back to step 6
Many thanks for your contribution, rigor and patience!
In the open source world sometimes it happens that people work on a feature for weeks or month and leave after it has been finished for 98%. Although this 2% don't look like a big issue, usually that means that all your work doesn't make it into the production code. Normally, other developers are busy and don't have the time to dive into your work and find the pieces that are missing or not working yet. This situation would be a waste of your precious time. We bet that it is way more satisfying if your contribution makes it into production and will be used by scientists around the world.
For a contribution to be considered 100% complete, it must (be)
- comply to our coding guidelines,
- unit tested,
- well documented
- include a demo / screencast in certain cases.
Therefore, we want to encourage you to reserve enough time in the end where you don't code. We do our best to support you!
make check-submission is our central automatic tool to validate if code changes are ready to be merged into master. It performs a series of checks and reports errors or warnings.
Therefore, it makes the code review process easier. Since developers can execute it on their local machine, the feedback loop is much tighter, resulting in a faster submission process. Although, many issues are caught, it has its limitations. Thus, it cannot fully replace manual code reviews.
Since it possibly outputs a lot of information, this page explains how to interpret the results.
Here an example how the output should look like if all checks are OK
- Successful build without compiler warnings
- All tests pass
clang-formatdoes not report issues
clang-tidydoes not report issues
cpplintdoes not report issues
- doxygen does not report issues
Here an example of a passing build, but with issues in many categories -- these issues must be fixed as well: