Code Standards and Guidelines

Nova Code Standards and Guidelines, v1.0

October 29, 2021

Note: throughout this document, I will mention team members by their position rather than by name. This is so that in the future, when these positions change, this document will still be valid. At the time of writing, the team lead is Will Heitman, and the software architect is Joshua Williams. In general, questions about this document should be directed to one of them, and preferably the software architect.

These are the guidelines that we should follow when writing our code. If for some reason these don't apply to some part of the project, talk to the software architect or team lead before making an exception yourself.

1. Source Control

• (1a) Use the branching strategy described below. Do not commit directly to the main branch. This applies even to simple changes, like fixing a typo!

• (1b) Don't merge your own pull requests to main, dev or release branches. Get the software architect or team lead to review your change and merge it.

• (1c) Commit as often or as infrequently as you like. As long as you're working in a branch (as described below), you can commit even a completely broken change without affecting others.

Branching Strategy

We will maintain both a main branch and a dev branch. main is more stable than dev. When you begin work on a feature or a bugfix, fork the dev branch, implement your change, and create a pull request back to dev once the change is relatively stable and well tested.

Please start branch names with 'feature_' or 'bugfix_', but other than that, name them however you want. Make the name descriptive and helpful! Don't name a branch feature_zcc - that name is not helpful to someone trying to understand what the branch is for. However, feature_zed_camera_color clearly describes what the branch is for.

When we are preparing for a release, we will create a branch off of dev to test and and add bugfixes. This release will start with release_ and end with a version number - for example, release_1_0. We can continue merging features into dev while a release is being worked on! Once testing is complete, the release branch will be merged into both main and dev, so that the fixes applied during testing are applied everywhere. The commit in main will be tagged as our new release.

2. Documentation

• (2a) Our best documentation is the code itself. Give things nice, descriptive names! Readability is extremely important in a project like this. Constantly refactor your code so it is understandable to others.

• (2b) Any part of the code that isn't immediately obvious should have comments explaining what is being done.

• (2c) Each file should have a comment at the top of the following format:

/*
 * Package:   package_name
 * Filename:  FileName.cpp
 * Author:    John Doe
 * Email:     [email protected] (Use either school or personal)
 * Copyright: 2021, Nova UTD
 * License:   MIT License
 */

• (2d) Except in trivial cases, every header file should have further comments explaining the purpose of the class defined within.

• (2e) Before a pull request is merged, please document what changes you've made in comments on the pull request.

• (2f) If a pull request makes a major change, the wiki should be updated immediately when it is merged.

• (2g) Issues on ZenHub should be marked as "complete" only after the request is merged and the documentation has been updated.

3. Testing

• (3a) When running tests, ideally every part of the code will execute. This is known as "code coverage".

• (3b) While our project does not currently use a code coverage analysis tool, code coverage measurements will help you evaluate whether you have really checked all of the cases that need checking. We may find a coverage analysis tool in the future.

• (3c) Tests should, in general, cover all of these categories: basic startup, simple operation under light load, normal operation, complex operation with heavy load, and error cases (invalid input, etc). This means that, when something fails, we will immediately get some pointers as to what has gone wrong.

• (3d) If the situation is trivial, talk to the software architect about appropriate testing. In general, we want to test just enough that we can be certain that the code will execute properly, but we also don't want to waste time writing redundant tests. That said, if it's easy to test, adding extra tests will do no harm!

• (3e) Use googletest to write your tests, as in our existing code. This allows colcon to easily test our entire codebase.

• (3f) The voltron_test_utils package is available to allow you to test your code at the ROS node level. Please do not rewrite this code - use what we already have.

4. Coding Practice and Style

• (4a) Don't reinvent the wheel! If you can use existing code rather than writing your own, you usually should. This saves time and often results in using more mature code than we could write ourselves.

• (4b) Prefer readability over performance in most code. The obvious exception to this is code that is going to process a large amount of data (ie, the inner loop of a downsampler). However, much of our code runs relatively infrequently, so small performance gains are not worth obfuscating our code.

• (4c) C++ source files should use a .cpp extension. Header files should use a .hpp extension.

• (4d) Follow the style of our existing code. Curly braces don't need their own line. Variables and function names are written in snake_case. Class names are written in CamelCase. Everything is nested under the navigator::package_name namespace.

• (4e) See the "Documentation" header for guidelines concerning comments!

• (4f) Names should be meaningful, even if it makes them very long. Good names are the #1 factor in writing readable code! Do NOT write "int inchs = read(sa, b BUFSZ);" - instead write "int characters_read = read(socket, buffer, buffer_size)".

• (4g) If you need to work with low-level OS functionality (eg, CAN interfaces), either isolate this part of the code in a safe, RAII (Resource Acquisition Is Initilization - look this up if you're unfamiliar with it), C++-style class, or use a library that provides this same functionality. Don't scatter the magic system calls, manual memory management, etc throughout your code.

• (4h) If you end up writing a general-purpose utility of some sort, write it in its own package so that other packages can require it later on (see voltron_test_utils for an example).

• (4i) Lots of general coding practice guidelines can be found here: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#main. In general, we should follow these in our code. While you don't have to read them all immediately, it's recommended that you work through these over the rest of the semester. You don't have to memorize them, just be familiar with the general concepts.

5. Miscellaneous Guidelines

• (5a) If you need to merge a "quick and dirty" change (in unusual circumstances only), you may make a change and create a pull request without testing, documentation, etc. In this case, don't delete the branch and don't mark the task as done in ZenHub! Keep working on tests and documentation in the same branch. Once the code looks good, create a new pull request as per the guidelines above.

• (5b) If you need to add an external dependency, talk to the software architect or team lead about it first. The answer will almost certainly be "yes", but it's important to track these things to keep our code from becoming spaghetti. Don't let this stop you from using existing tools whenever they are available!

• (5c) Don't hesitate to ask questions! Any question about the software can be directed to the software architect. When in doubt, better to ask and do it right the first time than to have to redo it.