Chapter 7 Contributor Guidelines
External contributions and feedback are important to the development and future maintenance of FIMS and are welcome. This section provides contributing guidelines and workflows for developers of FIMS and the ecosystem surrounding FIMS. All contributors, both internal and external, are required to abide by the Code of Conduct.
7.1 Code
7.1.1 Style Guide
Style guides are used to ensure the code is consistent, easy to use (e.g., read, share, and verify) and ultimately easier to write.
7.1.1.1 C++
We use the Google C++ Style Guide for C++ code.
7.1.1.2 R
We use the tidyverse style guide for R code.
7.1.2 Coding Good Practices
Following good software development and coding practices simplifies collaboration, improves readability, and streamlines testing and review. The following are industry-accepted standards:
- Adhere to the adopted Style Guides
- Avoid rework - take the time to check for existing options (e.g., in-house, open source, etc.) before writing code
- Keep code as simple as possible
- Use meaningful variable names that are easy to understand and clearly represent the data they store
- Use descriptive titles and consistent conventions for class and function names
- Use consistent names for temporary variables that have the same kind of role
- Add clear and concise coding comments
- Use consistent formatting and indentation to improve readability and organization
- Group code into separate blocks for individual tasks
- Avoid hard-coded values to ensure portability of code
- Follow the “Don’t Repeat Yourself” (DRY) principle for coding
- Avoid deep nesting
- Limit line length (wrap ~72 characters) of code but use a single line per paragraph in this markdown document
- Capitalize SQL queries so they are readily distinguishable from table/column names
7.2 Roadmap to FIMS File Structure and Organization
7.2.1 Files that go in inst/include
7.2.1.1 common
This folder includes files that are shared between the interface, the TMB objective function, and the mathematics and population dynamics components of the package.
7.2.1.4 population_dynamics
Each folder in population_dynamics corresponds to a component of the population dynamics model. Given the complexity of the component, the structure in these folders within population_dynamics may differ. At a minimum there will be a .hpp file with the same name as the subfolder, e.g., fleet/fleet.hpp. This file will include an ifndef directive and #include statements. If the folder is empty other than this file, then the remainder of the file will define the component. If there are additional subdirectors along side the .hpp file, the file will end after the include statements that point to each of the files in the functions folder that sits next to this .hpp file.
For the latter scenario, where a functor folder exists, inside the functor folder will be a .hpp file with _base attached to the component name, e.g., population_dynamics/maturity/functors/maturity_base.hpp. This _base.hpp file will define the base class for the module type. The base class should only need a constructor method and a number of methods (e.g., evaluate()) that are not specific to the type of functions available under the subfolders but reused for all objects of that class type.
7.3 GitHub Collaborative Environment
Communication is managed via the NOAA-FIMS GitHub organization.
7.3.1 GitHub Issues
GitHub Issues are key to keeping everyone informed and prioritizing tasks. All bugs, future projects, ideas, concerns, developments, etc. must be documented using an issue the appropriate repository’s Issue Tracker within the NOAA-FIMS organization, e.g., Issue Tracker for NOAA-FIMS/FIMS, before the code is altered. Issues are automatically tagged with the status: triage_needed tag and placed on the Issue Triage Board where they will be labeled, given an assignee, and given a milestone by those in charge of the triage process.
There are different types of issues, e.g., bug, enhancement, etc., that might be applicable to your situation. Please read the descriptions for each issue type available and use the form that best suits your situation. The available types are the same or similar on each GitHub Issue page within the NOAA-FIMS organization. Navigate to the GitHub Issue page for NOAA-FIMS/FIMS to see the available types.
7.3.1.1 GitHub Issue Labels
Labels help categorize and manage the work ahead of us. Members of the NOAA-FIMS organization can add labels to their issues upon submission of the issue. Additional labels will be applied during the triage process if needed.
Labels within NOAA-FIMS are categorized, e.g., status: triage_needed is a status label. Typically, every issue should have one label per category.
7.3.1.2 GitHub Issue Templates
The templates for issue types are maintained in the .github/ISSUE_TEMPLATE folder within the NOAA-FIMS/.github repository, which allows the same forms to be available in each repository without copying and pasting the code to each repositories .github/ISSUE_TEMPLATE folder. Additional examples of Example templates for issues and pull requests can be found on GitHub Docs.
7.3.1.3 GitHub Projects
GitHub Projects are used to keep track and prioritize issues. There are several GitHub Projects for NOAA-FIMS such as the Issue Triage Board.
7.3.2 GitHub Cloning
Any repository within NOAA-FIMS can be cloned to a personal or virtual machine. But, contributors without write access will need to fork the repository and then clone their fork.
7.3.3 GitHub Branching
After cloning and before any work is done, ideas for potential changes to the code should be proposed in a GitHub Issue. Next, after the issue is filed and before any work is done, a feature branch should be created where the work will be done.
7.3.3.2 Branching Strategy
FIMS uses a Scaled Trunk-Based Development branching strategy to streamline the integration of new code into the trunk. This strategy was mainly chosen for the following two reasons:
- Encourage small, frequent changes.
- Enable the release of bug fixes without new features.
Some branches within NOAA-FIMS repositories have branch protection rules. For example, the branch protection rules for the main branch of NOAA-FIMS/FIMS are described on GitHub in the branch settings.
7.3.4 GitHub Pull Requests
Once development within a feature branch is complete, a Pull Request (PR) should be initiated. PRs can be initiated with a draft status or marked as ready for review. The former will allow the code to be tested using tests available in GitHub actions without initiating a formal review. The latter will initiate the code review process. For repositories/branches with branch protection rules, the PR must be formally reviewed, approved, and pass all tests (see the section on GitHub Actions) before the branch can be merged into the chosen branch.
Instructions for submitting your PR are available on the PR form on GitHub. PRs from forks are treated the same as PRs from within the repository, see the GitHub documentation on creating a PR from a fork for more information.
7.3.4.1 Pull Request Templates
The templates for PRs are maintained in the .github/folder within the NOAA-FIMS/.github repository, same as NOAA-FIMS Issue templates.
See the NOAA-FIMS/FIMS PR template here.
7.3.4.2 Pull Request Review
Code review ensures health and continuous improvement of the FIMS codebase, while simultaneously helping FIMS developers become familiar with the codebase. Tools within GitHub allow for reviewers to analyze code changes, provide inline comments, and view change histories with ease.
Google’s code review guide provides a useful set of guidelines for both reviewers and code authors.
Below is a flowchart for the FIMS code review process. The author starts by submitting a PR, ensuring documentation, tests, and continuous integration checks are complete. The initiator of the PR must propose a reviewer (see Assigning Reviewers). The reviewer receives the review request and either executes the review independently or pairs with another team representative if assistance is needed. Based on the review, changes may be requested, which the author must address before approval. Once the PR is approved, the author must rebase and merge it into the desired branch.
7.3.4.2.1 Assigning Reviewers
Reviewers of PRs for changes to the codebase in FIMS should be suggested by the author of the PR. For those FIMS Implementation Team Members that keep their status in GitHub current (see “Setting a status” for more information), authors of PRs can use the status information to prevent assigning a reviewer who is known to be “Busy”.
If a review has been assigned to you and you don’t feel like you have the expertise to address it properly, please respond directly to the PR so a different reviewer can be found promptly or suggest someone to help you with the review.
7.3.4.2.2 Review Checklist
While automated testing can assure the code structure and logic pass quality checks, human reviewers are required to evaluate things like functionality, readability, etc. Every PR is accompanied by an automatically generated checklist of major considerations for code reviews. Additional guidance is provided below for reviewers to evaluate when providing feedback on code:
7.3.4.2.3 Reviewer Good Practices
Good reviews require good review habits. Please use the guidance found at Conventional Comments for formatting/structuring your reviewer comments. Navigate to the Perforce Blog for nine best practices for code review and use the FIMS Style Guide to settle any style arguments.
7.3.4.3 Merge Conflicts
Merge conflicts must be resolved before a PR can be merged into the desired branch because Git cannot automatically determine which version of the code should be kept.
Merge conflicts can be resolved on GitHub or locally using Git.
Tools such as Git Kracken or GitLens within VS Code offer a GUI interface to help resolve merge conflicts. Tatiana Tylosky’s guide to merge conflicts is also helpful.
7.3.5 GitHub Actions
FIMS uses GitHub Actions to automate routine tasks such as testing, building and deploying websites, and notifying individuals. Some of the actions are built on reusable workflows available in {ghactions4r}, where reusable workflows allow for sharing of code.
For the repositories in NOAA-FIMS that use GitHub actions, the actions can be found in .yml files that are stored in the .github/workflows directory. For example YAML files in .github/workflows of the NOAA-FIMS/FIMS repository specify the setup for the GitHub Actions for the source code of FIMS.
7.3.5.1 Example Actions
- call-r-cmd-check runs R CMD Check on the FIMS R package using the current version of R across three operating systems, Windows, Linux (Ubuntu), and OSX. R CMD Check ensures that the FIMS package can be downloaded without error. To replicate the GitHub Actions workflow locally, use
devtools::check(). - run-clang-tidy runs checks while compiling the C++ code. If this run fails, fixes need to be made to the C++ code to address the issue identified.
- run-googletest runs the GoogleTest C++ unit tests and benchmarking. If this run fails, then fixes need to be made to the C++ code and/or the GoogleTest C++ unit tests. To replicate this GitHub Actions workflow locally, follow instructions in the testing section.
7.3.5.2 GitHub Actions Results
Completed instances of the GitHub Actions can be viewed by navigating to the Actions tab a repository. For example, the NOAA-FIMS/FIMS/actions. The status of GitHub Actions applicable to a PR can be viewed on the GitHub PR after the PR is initiated or after any commit is pushed to a PR.
7.3.5.3 Debugging Broken GitHub Actions
GitHub Actions can fail for many reasons, so debugging is necessary to find the cause of the failing run. The tips found below can be helpful for determining why the action failed.
- Ask for help as needed! Some members of the FIMS team who have experience debugging GitHub Actions are Bai, Kathryn, and Ian.
- Investigate why the run failed by looking at the log file. GitHub provides some guidance on what is contained in the log file.
- Try to replicate the problem locally. For example, if the call-r-cmd-check run fails during the testthat tests, try running the testthat tests locally (e.g., using
devtools::test()in an R session). - If the problem can be replicated, try to fix locally by fixing one test or issue at a time. Then push the changes up to GitHub and monitor the new GitHub Action run.
- Check if the problem is present across all investigated operating systems because it might be operating system specific and not reproducible on your machine. For example, if using Windows locally, it may be an issue specific to Mac or Linux.
- Sometimes, runs may fail because a particular dependency was not available at the exact point in time need for the run (e.g., maybe R did not install because the R executable could not be downloaded); if that is the case, wait a few hours to a day and try to rerun. If it continues to fail for more than a day, a change in the GitHub Action YAML file may be needed.
7.3.6 GitHub Teams
GitHub Teams can help organize members into groups with each group having certain permissions. Teams for NOAA-FIMS are not currently used.
7.4 git
git is a distributed version control system that tracks files. All files within NOAA-FIMS are tracked using git and GitHub (see the section on GitHub) is used as a central server. The use of git stops the need for saving files with file_1.txt and file_2.txt.
7.4.1 git Example
- Use the following commands to (a) move to the local branch that you want your feature branch to be based off of, e.g., main in the example below; and (b) create a new branch, where you will replace
with the name of your feature branch (see Branch Naming Conventions) and check it out.
- Periodically rebase your feature branch with the branch you plan on merging into in the future by (a) fetching all the changes stored in GitHub for the branches in your clone and (b) rebasing your feature branch, e.g., here we rebase to origin/main because we eventually want to merge into main. We can rebase to any remote or local branch though. Be careful when rebasing if you have already pushed your feature branch to GitHub because rebasing will rewrite your commit hashes in your feature branch.
- While editing code, commit regularly following commit messages guidelines by (a) adding the changed files to the que and (2) committing them to the feature branch with a thoughtful commit message. If you configure your editor for git you can remove the
-margument and use a text file to make a more meaningful commit message with multiple lines.
- Consider interactively rebasing your commits to squash similar commits into a single commit or rearrange the order of your commits. The following example is for the situation where you want to look at the last ten commits.
- Push your committed changes to GitHub.
When you are finished making changes to your feature branch, navigate to GitHub and open a pull request to the desired branch, e.g., main in this example.
Once the PR is approved and merged into the desired branch you (a) switch to a different branch and (b) delete the local version of your feature branch. Note that you might have to use
-Dinstead of the lower-case version if you want to force the removal of the branch given that we use a rebase strategy and the commit hashes might have changed, thus git to think that the feature branch has not been merged into main because your local version of the feature branch might have different hashes than the remote version.
7.4.2 Commit Messages
Commit messages communicate details about changes that have occurred to collaborators and improve team efficiency. The best guidance for how to create an excellent commit message can be found on Conventional Commits, which is our style guide for commit messages.