Contribution Workflow

Jira for Project Management

Jira is a project management tool that has an emphasis on agile development. NUbots use Jira to document issues and projects, plan which tasks to prioritise, manage who is responsible for tasks, and keep track of the progress of tasks. You should ask the team leader or a senior team member to add you to the Jira site if you do not have access.

Each subgroup at NUbots has its own Kanban-style Jira board. While most tasks fit neatly in one project board, some may be harder to place - in this case, the most relevant board is used. Each board contains tasks that are grouped within Epics - an Epic is a particular theme, such as 'reinforcement learning' or 'subcontroller design'. Not all tasks live within an Epic, but organising related tasks under a single Epic can aid in visualisation and organisation. The Epic-Task paradigm also encourages breaking a larger project down into actionable and manageable chunks, particularly when multiple people are working on the same project.

A NUbots Kanban board has five columns:

• Backlog: Tasks that have been identified, but are not currently a priority.
• Selected For Development: The next tasks to work on.
• In Progress: Tasks that are being actively worked on.
• In Review: Tasks that are in a GitHub PR and awaiting approval, or awaiting hardware approval by a senior member.
• Done: Tasks that have been completed. Tasks in this column are automatically cleaned up after two weeks.

Each board is visualised with swimlanes - distinct rows separated by Epics. A 'no Epic' swimlane can be found at the bottom for tasks that don't fit inside any Epics.

Tasks are typically assigned by the team leader or a senior team member during planning and meetings, however team members can also volunteer for tasks by assigning themselves to the task in Jira if no one else is working on it. Team members are encouraged to move their own tasks through the columns, but this can also be done by the main project managers on the team.

If you identify an issue or a good project to include in Jira, add it to the backlog column in the relevant Kanban board and Epic (or the 'no Epic' swimlane if nothing fits).

Prerequisites for Git

This guide assumes you know the basics of using a command line (also called a terminal). Specifically, it assumes you know how to navigate around the filesystem and run commands. You can learn how to use the command line in this article. If you are on Windows, you should install WSL to use the Bash terminal, since many of the commands you will be using are not available in the default Command Prompt.

It also assumes you know the basics of using Git, such as cloning a repository, adding and committing changes, and pushing your changes to a remote. If you don't, you can learn about that in this short introduction to Git.

Getting Code Into a Repository

Let's suppose you have created a file which you would like to add to a NUbots repository called MakeRobotStand.cpp, which is part of your project to make the robot stand up.

In the terminal, change directory to the location where the repository is cloned.

Select Branch

If you are working within an existing project, then you need to find out which branch the work is on. To list the branches, use the command git branch -r. When you have found the branch you want to push to, swap to it using the following command:

Copy
git checkout <branch name>

Where <branch name> corresponds to the branch you found in the last step.

If you are not working on an existing project, you need to make a branch to commit your changes to. The format for branch names is your_surname/brief_description_of_project. So if your surname is Hamiltons, and your project is making the robot stand up, a good branch name would be hamiltons/stand_up.

Making a Pull Request

Once you have a set of commits on your branch which you think should be added to the main codebase, you can make a Pull Request (PR). PRs are where users ask the repository to pull their branch into the main branch.

GitHub has documentation about PRs here.

The title of your PR is important. It will determine what the eventual commit message will be that others will read to know what has changed. Title your PR with a description of the changes that it will make. For this example we could use Add a stand-up script. If you need to give more detail about what the code does, put that as a comment. If done properly, your PR title should finish the sentence "When merged, this PR will <TITLE HERE>".

To be approved, you should add reviewers to the pull request. Each reviewer will receive a notification. If you have a project mentor, they should be a reviewer, and at least one other person who knows about that part of the codebase or the problem you are trying to solve. Once the reviewers have approved the changes, the changes can be merged into the main branch.

Code Review

The reviewers will give constructive feedback about your code, by making comments on code snippets from your changes. You can respond to these on GitHub. They can also directly suggest changes to specific sections. Additional commits you make to the branch will update the PR and will reset reviews of the changed sections.

Once all of the comments are resolved, the reviewers have approved the PR, and the build checks have passed (see Formatting and PR Checks), you can Squash and Merge. Squashing combines every commit on the PR into a single commit so that the main branch history is clean and easy to follow. Once the code has been squashed and merged, it is in the codebase and you have succeeded in contributing code to NUbots.

Commit Etiquette

Commits should make minor changes, and the changes in each commit should have a common theme. A general mantra for contributing code is commit often, push once, but this isn't strict at all. Committing often means that the code is built up incrementally. Pushing once means that you don't spam all the other contributors with notifications.

Commit messages should describe what you did, starting with an action verb in the present tense. If you added a file with a new stand script, for example, prefer "Add new stand script" over "Added new stand script" or "Adding new stand script" or "Add script".

Formatting and PR Checks

Before making a pull request, it is important that your code adheres to the code style and formatting rules for the project you are contributing to.

• For code in the NUbots repository, run ./b format to fix formatting issues before making a commit.
• For code in NUsight, run yarn eslint:fix to fix formatting issues before making a commit.
• For code in NUbook, run yarn format to fix formatting issues before making a commit.

If your code is in a different repository and it uses C++ or protobuf files, use the .clang-format file in the NUbots repo to format them.

If you push your changes to GitHub and get a message that the build failed due to formatting issues, try running the relevant format commands, commit any changes, and push to GitHub again. This will trigger a new build, which will check the formatting again. You won't be able to merge the PR until the build checks pass.

Work in Progress PRs

If you have an incomplete project you would like feedback on, you can make a draft PR to get your code reviewed. Prepend [WIP] to the PR title when making draft pull requests. Draft PRs cannot be merged, so there is no need to worry about merging in bad code. GitHub has documentation about draft PRs here.

Reviewing Code

To review code on GitHub, go to the relevant PR page and open the Files Changed tab. This will show the diff of the PR.

You can add comments and/or suggestions to one or more lines in the diff. For a single line, move your mouse cursor over the line and click the plus button that appears next to the line number. For multiple lines, click and drag the plus button down, starting on the first line. A Markdown editor will pop up to write your comment or suggestion. To make a code suggestion, click the button that looks like a file icon with ± inside it, and then make your changes to the code snippet that is inserted. Click the Preview tab to preview your changes. When done, you can either leave a single comment or start a review. In a review, all comments you make will be pending until you finish your review, when they will all be posted at once.

Try to be as clear as possible when making comments. The contributor should be able to understand if the comment is asking for clarification, or asking for changes. Be kind and make sure to word your comments constructively. Prefer "we" over "you" when making suggestions for what to do.

When reading the code, if there is anything you find confusing or potentially incorrect, make a comment with all your concerns. For example, if there is a nontrivial block of code without a comment explaining it, you should ask the author to add a comment. Another example is if there is a block of code that has a side effect that the author may not have considered. You should inform the author of such and suggest an alternative if possible.

When reviewing documentation PRs, you should look for factual inaccuracies, omissions of important information, awkward sentences, and typos.

After reading the code and adding your pending comments, click Finish your review at the top right. This will show a popup with an input for overall comments, as well as the three actions you can take to finish your review:

• Request changes: this blocks the code from being merged until you subsequently approve the PR in another review, even if others have approved it. Your review may be dismissed, but this is generally discouraged.
• Approve: this allows the PR to be merged, so don't do this unless you are reasonably confident that the changes are correct.
• Comment: this adds your comments without blocking or approving the changes. Do this if you are not too concerned about the comments you have made, or if you're not sure when you'll be able to approve the PR once the changes are made.

Reviewing code that you may not be fully confident with and leaving comments asking for clarification helps you learn and helps the author write more readable code, which is better for everyone.

Writing Reviewable Code

Writing reviewable code is a skill that can be developed with practice. Here are some general guidelines to make your PR easier to review:

• Make it atomic: A reviewable PR should only address one issue or add one feature.
• Keep it small: Ideally, a reviewable PR should not change much more than 400 lines of code, anything above this super-exponentially increases the time it takes to review, and makes it more likely that an error will be missed.
• Name it well: A reviewable PR should be named starting with a verb saying what you did, what issue was fixed, or what feature added. Prefer the present tense, e.g. "Update dependency X to version Y" instead of "Updated dependency X to version Y".

To keep PRs small and atomic you may have to split your PR into multiple sub PRs. To do this, make a new branch branch from the original PR and move over some code. Then make a separate PR targeting the original PR's branch. Make sure to mention the original PR in the new PR's description, using #123, where 123 is the sub PR's number. After completing the sub PR, merge it back into the original PR.

Handling Reviews and Reviewee Etiquette

Here are some tips for handling reviews as the author of a PR:

• When a review comment is left on your PR, you may respond with a reply on GitHub, or with a comment in the code. Prefer comments in the code where appropriate, so in the future others don't have to read through several PRs and can instead just read the code.
• When applying suggested changes, you can go to the Files Changed tab and see the comments with the context of the code. Here you can also batch apply suggestions, so they only take one commit.
• After addressing a review comment, you can click Resolve Conversation to hide the conversation thread and reduce visual clutter. It is the reviewer's responsibility to check that your changes adequately addressed their concerns.
• Don't be alarmed when a reviewer makes over a dozen comments on your PR, just work through them systematically, and don't hesitate to ask for help or clarification if you need more details.
• Finally, when all the comments have been addressed from a review that requested changes, you can request another review by clicking the Re-request review button next to the reviewer's name.