Chapter 7 Contributor Guidelines

External contributions and feedback are important to the development and future maintenance of FIMS and are welcome. This section provides guidelines and workflows for FIMS developers and collaborators on how to contribute to the project.

7.1 Style Guide

The FIMS project uses style guides to ensure our code is consistent, easy to use (e.g. read, share, and verify), and ultimately easier to write. We use the Google C++ Style Guide and Google’s R Style Guide. Google’s R Style Guide is based off of the tidyverse style guide, with a few minor modifications to improve readability and portability.

7.2 Naming Conventions

The FIMS implementation team has chosen to use typename instead of class when defining templates for consistency with the TMB package. While types may be defined in many ways, for consistency developers are asked to use Type instead of T to define Types within FIMS.

7.3 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 FIMS Project style guide
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 DRY principle - “Don’t Repeat Yourself” (or your code)
Avoid deep nesting
Limit line length (wrap ~72 characters)
Capitalize SQL queries so they are readily distinguishable from table/column names
Lint your code

7.4 Roadmap to FIMS File Structure and Organization

7.4.1 Files that go in `inst/include`

7.4.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.4.1.2 interface

This includes the R interface files.

7.4.1.3 population dynamics

There are subfolders underneath this folder that correspond to the different components of the population dynamics model. Each of the modules will need a .hpp file that only consists of #include statements for the files under the subfolders.

In the subfolder, there will need to be one file called _base.hpp that defines 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.4.2 Files that go in `src/`

7.4.2.1 FIMS.cpp

This is the TMB objective function.

7.5 GitHub Collaborative Environment

Communication is managed via the NOAA-FIMS Github organization.

New features requests and bugs should be submitted as issues to the FIMS development repo. For guidelines on submitting issues, see Issue Tracking.
GitProjects TODO: add description * GitHub Teams TODO: add description * All contributers, both internal and external, are required to abide by the Code of Conduct

7.5.1 FIMS Branching Strategy

There are several branching strategies available that will work within the Git environment and other version control systems. However, it is important to find a strategy that works well for both current and future contributors. Branching strategies provide guidance for how, when, and why branches are created and named, which also ties into necessary guidance surrounding issue tracking.

The FIMS Project uses a Scaled Trunk Based Development branching strategy to make tasks easier without compromising quality.

Scaled Trunk Based Development; image credit: https://reviewpad.com/blog/github-flow-trunk-based-development-and-code-reviews/

This strategy is required for continuous integration and facilitates knowledge of steps that must be taken prior to, during, and after making changes to the code, while still allowing anyone interested in the code to read it at any time. Additionally, trunk-based development captures the following needs without being overly complicated:

Short-lived branches to minimize stale code and merge conflicts * Fast release times, especially for bug fixes * Ability to release bug fixes without new features

7.5.2 Branch Protection

Branch protection allows for searching branch names with grep functionality to apply merging rules (i.e., protection). This will be helpful to protect the main/trunk branch such that pull requests cannot be merged in prior to passing various checks or by individuals without the authority to do so.

7.5.3 GitHub cloning and branching

For contributors with write access to the FIMS repo, changes should be made on a feature branch after cloning the repo. The FIMS repo can be cloned to a local machine by using on the command line:

git clone https://github.com/NOAA-FIMS/FIMS.git

7.5.4 Outside collaborators and forks

Outside collaborators without write access to the FIMS repos will be required to fork the repository, make changes, and submit a pull request. Forks are discouraged for every-day development because it becomes difficult to keep track of all of the forks. Thus, it will be important for those working on forks to be active in the issue tracker in the main repository prior to working on their fork — just like any member of the organization would do if they were working within the organization. Knowledge of future projects, ideas, concerns, etc. should always be documented in an issue before the code is altered.

Pull requests from forks will be reviewed the same as a pull request submitted from a branch. Users will need to conform to the same standards and all contributions must pass the standard tests as well as have tests that check the new feature.

To fork and then clone a repository, follow the Github Documentation for forking a repo. Once cloned, changes can be made on a feature branch. When ready to submit changes follow the Github Documentation on creating a pull request from a fork

7.6 Issue Tracking

Use of the GitHub issue tracker is key to keeping everyone informed and prioritizing key tasks. All future projects, ideas, concerns, development, etc. must be documented in an issue before the code is altered. Issues should be filed and tagged prior to any code changes whether the change pertains to a bug or the development of a feature. At a minimum, all issues will be labeled with a future version number. Bugs with immediate fixes will be assigned to the current version number augmented for a hot fix and development will be based on code in the trunk. All other issues will be assigned to a future version and development will be based on version branches. That is, changes to the code for version 3.3 cannot start until there is a branch for version 3.3. This will minimize stale code and large merge conflicts.

7.7 Reporting Bugs

This section guides you through submitting a bug report for any toolbox tool. Following these guidelines helps maintainers and the community understand your report, reproduce the behavior, and find related reports.

7.7.0.1 Before Submitting A Bug Report

Check if it is related to version. We recommend using sessionInfo() within your R console and submitting the results in your bug report. Also, please check your R version against the required R version in the DESCRIPTION file and update if needed to see if that fixes the issue.
Perform a cursory search of issues to see if the problem has already been reported. If it has and the issue is still open, add a comment to the existing issue instead of opening a new one. If it has and the issue is closed, open a new issue and include a link to the original issue in the body of your new one.

7.7.0.2 How Do I Submit A (Good) Bug Report?

Bugs are tracked as GitHub issues. Create an issue on the toolbox Github repository and provide the following information by following the steps outlined in the reprex package. Explain the problem and include additional details to help maintainers reproduce the problem using the Bug Report issue template.

Provide more context by answering these questions:

Did the problem start happening recently (e.g. after updating to a new version of R) or was this always a problem?
If the problem started happening recently, can you reproduce the problem in an older version of R? What’s the most recent version in which the problem doesn’t happen?
Can you reliably reproduce the issue? If not, provide details about how often the problem happens and under which conditions it normally happens.
If the problem is related to working with files (e.g. reading in data files), does the problem happen for all files and projects or only some? Does the problem happen only when working with local or remote files (e.g. on network drives), with files of a specific type (e.g. only JavaScript or Python files), with large files or files with very long lines, or with files in a specific encoding? Is there anything else special about the files you are using?

Include details about your configuration and environment:

Which version of the tool are you using?
What’s the name and version of the OS you’re using?
Which packages do you have installed? You can get that list by running sessionInfo().

7.8 Suggesting Features

This section guides you through submitting an feature suggestion for toolbox packages, including completely new features and minor improvements to existing functionality. Following these guidelines helps maintainers and the community understand your suggestion and find related suggestions.

Before creating enhancement suggestions, please check the issues list as you might find out that you don’t need to create one. When you are creating an enhancement suggestion, please include an “enhancement” tag in the issues.

7.8.0.1 Before Submitting A Feature Suggestion

Check you have the latest version of the package.
Check if the development branch has that enhancement in the works.
Perform a cursory search of the issues and enhancement tags to see if the enhancement has already been suggested. If it has, add a comment to the existing issue instead of opening a new one.

7.8.0.2 How Do I Submit A (Good) Feature Suggestion?

Feature suggestions are tracked as GitHub issues. Create an issue on the repository and use the Feature Request issue template.

7.8.1 Issue Labels

Utilize labels on issues:

To describe the kind of work to be done: bug, enhancement, task, discussion, question, suitable for beginners
To indicate the state of the issue: urgent, current, next, eventually, won’t fix, duplicate

7.8.2 Issue Templates

Templates are available and stored within each repository to guide users through the process of submitting a new issue. Example templates for issues can be found on GitHub Docs. Use these references and existing templates stored in .github/ISSUE_TEMPLATE for reference when creating a new template.

7.9 Branch Workflow

This section details the workflow to create a branch in order to contribute to FIMS.

7.9.1 Branching Good Practices

The following suggestions will help ensure optimal performance of the trunk-based branching strategy:

Branches and commits should be kept small (e.g. a couple commits, a few lines of code) to allow for rapid merges and deployments.
Use feature flags to wrap new changes in an inactive code path for later activation (rather than creating a separate repository feature branch).
Delete branches after it is merged to the trunk; avoid repositories with a large number of “active” branches.
Merge branches to the trunk frequently (e.g. at least every few days; tag as a release commit) to avoid merge conflicts.
Use caching layers where appropriate to optimize build and test execution times.

7.9.2 Branch Naming Conventions

Example: feat-issue-3-R-package-skeleton

When creating a branch, start the branch name with a group word:

bug: bug fix
feat: new feature

Use issue tracker ID in branch name
Include brief description
Use a hyphen as separators

7.9.3 git workflow

Use the following commands to create a branch:

$ git checkout -b <branchname> main           #creates a local branch
$ git push origin <branchname>                #pushes branch back to gitHub

Periodically merge changes from main into branch

$ git merge main                              #merges changes from main into branch

While editing code, commit regularly following commit messages guidelines

$ git add <filename>                          #stages file for commit
$ git commit -m"Commit Message"               #commits changes

To push changes to gitHub, first set the upstream location:

$ git push --set-upstream origin <branchname> #pushes change to feature branch on gitHub

After which, changes can be pushed as:

$ git push                 #pushes change to feature branch on gitHub

When finished, create a pull request to the main branch following pull request guidelines

7.10 Code Development

Code is written following the Style Guide, FIMS Naming Conventions, and Coding Good Practices

7.11 Commit Messages

FIMS Project contributors should provide clear, descriptive commit messages to communicate to collaborators details about changes that have occurred and improve team efficiency. Good commit messages follow the following practices:

Include a short summary of the change for the subject/title (<50 characters)
Include a blank line in between the ‘subject’ and ‘body’

Specify the type of commit:

    * fix: bug fix
    * feat: new feature
    * test: testing
    * docs: documentation
    * chore: regular code maintenance (e.g. updating dependencies)
    * refactor: refactoring codebase
    * style: changes that do not affect the meaning of the code; instead address code styling/formmatting
    * perf: performance improvements
    * revert: reverts a previous commit
    * build: changes that affect the build system

If the commit addresses an issue, indicate the issue# in the title
Provide a brief explanatory description of the change, addressing what and why was changed
Wrap to ~72 characters
Write in the imperative (e.g. “Fix bug”, not “Fixed bug”)
If necessary, separate paragraphs by blank lines
Utilize BREAKING CHANGE: <description> to provide expanation or further context about the issue being addressed.
If the commit closes an issue, include a footer to note that (i.e. “Closes #19”)

7.12 Merge Conflicts

7.12.1 What is a merge conflict?

A merge conflict happens when changes have occured to the same piece of code on the two branches being merged. This means Git cannot automatically determine which version of the change should be kept. Most merge conflicts are small and easy to figure out. See the Github Documentation on merge conflicts for more information.

7.12.2 How to prevent merge conflicts

Merge in small changes often rather than making many changes on a branch that is kept separate from the main branch for a long time.
Avoid refactoring the same piece of code in different ways on separate branches.
Avoid working in the same files on separate branches.

7.12.3 How to resolve merge conflicts

Merge conflicts can be resolved on Github or locally using Git.

An additional helpful resource is this guide to merge conflicts.

7.13 Pull Requests

Once development of a module is complete, the contributor must initiate a pull request. Github will automatically start an independent review process before the branch can be merged back into the main development branch. Pull requests are used to identify changes pushed to development branches. Open pull requests allow the FIMS Development Team to discuss and review the changes, as well as add follow-up commits before merging to the main branch. As noted in the branching stratgegy section, branches, commits, and pull requests should be kept small to enable rapid review and reduce the chance of merge conflicts. Any pull requests for the FIMS Project must be fully tested and reviewed before being merged into the main branch. Use the pull request template to create pull requests. Pull requests without this template attached will not be approved.

7.14 Code Review

Code review ensures health and continuous improvement of the FIMS codebase, while simultaneously helping FIMS developers become familiar with the codebase and ensure there is a diverse team of knolwedgable collaborators to support the continued development and maintenance of FIMS. CI/CD requires rapid review of all new/modified code, so processes must be in place to support this pace. FIMS code review will utilize tools available via GitHub, which allows reviewers to analyze code changes, provide inline comments, and view change histories.

The Google code review developer guide provides a useful set of guidelines for both reviewers and code authors.

7.14.1 Assigning Reviewers

Reviewers for the FIMS Project may be assigned in two different ways:

A specific member of the FIMS Development Team is requested to review a pull request, based on their specific expertise.
Code review assignments are automatically assigned using the GitHub load balance routing algorithm; this approach tries to ensure that each team member reviews an equal number of pull request in any 30 day period.

Team members should keep their status in Github current (see “Setting a status” for more information). Reviews will not be auto-assigned to “Busy” team members.

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 code owner immediately so a different reviewer can be found promptly.

7.14.2 Automated Testing

Automated testing provides an initial layer of quality assurance and lets reviewers know that the code meets certain standards. For more on FIMS testing, see Testing and GitHub Actions.

7.14.3 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 pull request 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:

Design (Is the code in the proper location? Are files organized intuitively? Are components divided up in a sensible way? Does the pull request include an appropriate number of changes, or would the code changes be better broken into more focused parts? Is the code focused on only requirements within the current scope? Does the code follow object- oriented design principles? Will changes be easy to maintain? Is the code more complex than it needs to be?)
Functionality (Does the code function as it is expected to? Are changes, including to the user interface (if applicable), good for users? Does parallel computing remain functional? How will the change impact other parts of the system? Are there any unhandled edge cases? Are there other code improvements possible?)
Testing (Does the code have appropriate unit tests? Are tests well- designed? Have dependencies been appropriately tested? Does automated testing cover the code exchange adequately? Could the test structure be improved?)
Readability (Is the code and data flow easy to understand? Are there any parts of the code that are confusing or commented out? Are names clear? Does the code include any errors, repeats, or incomplete sections? Does the code adhere to the FIMS Style Guide?)
Documentation (Are there clearl and useful comments available to why the code has been implemented as it has been? Is the code appropriately documented (doxygen and roxygen)? Is the README file complete, current, and adequately describe project/changes?)
Security (Does using this code open the software to possible security violations or vulnerabilities?)
Performance (Are there ways to improve on the code’s performance? Is there any complex logic that could be simplified? Could any of the code be replaced with built-in functions? Will this change have any impacts on system performance? Is there any debugging code that could be removed? Are there any optimizations that could be removed and still maintain system performance?)

7.14.4 Review Good Practices

Good reviews require good review habits. Try to follow these suggestions:

Review in short sessions (< 60 minutes) to maintain focus and attention to detail
Don’t try to review more than 400 lines of code in a single session
Provide constructive and supportive feedback
Ask open-ended questions and offer alternatives or possible workarounds
Avoid strong/opinionated statements
Applaud good solutions
Don’t say “you”
Be clear about which questions/comments are non-blocking or unimportant; likewise, be explicit when approving a change or requesting follow-up
Aim to minimize the number of nitpicks (if there are a lot, suggest a team-level resolution)
Use the FIMS Style Guide to settle any style arguments

7.15 Clean up local branches

If a code reviewer approves the pull request, FIMS workflow managers will merge the feature/bug branch back into the main repository and delete the branch. At this stage, the contributor should also delete the branch from the local repository using the following commands:

$ git checkout main           //switches back to main branch
$ git branch -d <branchname>  //deletes branch from local repository

7.16 GitHub Actions

FIMS uses GitHub Actions to automate routine tasks. These tasks include:

Backup checks for developers
Routine GitHub workflow tasks (not important for developers to monitor)

Currently, the GitHub Actions in the FIMS repository include:

GitHub Action Name	Description	Type	Runs a Check on PRs?	Runs on:
call-r-cmd-check	Runs R CMD Check	Backup Check	Yes	Push to any branch
run-clang-tidy	Checks for C++ code	Backup Check	Yes	Push to any branch
run-googletest	Runs the google C++ unit tests	Backup Check	Yes	Push to any branch
run-doxygen	Builds the C++ documentation	Backup Check	No	Push to main branch
run-clang-format	Styles C++ code	Routine workflow task	No	Push to main branch
call-doc-and-style-r	documents and styles R code	Routine workflow task	No	Push to main branch
pr-checklist	Generates a checklist as a comment for reviewers on PRs	Routine workflow task	No	Opening a PR

YAML files in a subdirectory of the FIMS repository specify the setup for the GitHub Actions. Some of the actions depend on reusable workflows available in {ghactions4r}.

Runs of the GitHub Actions can be viewed by navigating to the Actions tab of the FIMS repository. The status of GitHub Action runs can also be viewed on pull requests or next to commits throughout the FIMS repository.

7.16.1 Details on Backup Checks

Developers must make sure that the checks on their pull requests pass, as typically changes will not be merged into the main branch until all GitHub Actions are passing (the exception is if there are known reasons for the GitHub Actions to fail that are not related to the pull request). Other responsibilities of developers are listed in the Code Development section.

Additional details about the backup check GitHub Actions:

call-r-cmd-check runs R CMD Check on the FIMS package using the current version of R. Three runs occur simultaneously, on three operating systems: Windows, Linux (Ubuntu), and OSX. R CMD Check ensures that the FIMS package can be downloaded without error. An error means that the package cannot be downloaded successfully on the operating system for the run that failed. Developers should investigate the failing runs and make fixes. 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.16.2 Debugging Broken Runs

GitHub Actions can fail for many reasons, so debugging is necessary to find the cause of the failing run. Some steps that can help with debugging are:

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 in the log.
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()).
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.
If the problem cannot be replicated locally, it could be a operating specific issue; for example, if using Windows locally, it may be an issue specific to Mac or Linux.
Sometimes, runs may fail because a particular dependency wasn’t available at the exact point in time need for the run (e.g., maybe R didn’t install because the R executable couldn’t 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.16.3 How do I request a new Github Action workflow?

Routine actions and checks should be captured in a GitHub Action workflow in order to improve efficiency of the development process and/or improve automated checks on the FIMS codebase. New GitHub Action workflows can be requested by opening an issue in the FIMS repository.