07-contributor-guide.Rmd

# 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](#code-of-conduct).

## Code

### 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.

#### C++

We use the [Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html) for C++ code.

#### R

We use the [tidyverse style guide](https://style.tidyverse.org/) for R code.

#### Naming Conventions

We use `typename` instead of `class` when defining templates for consistency with the `TMB` package. While types may be defined in many ways, we use `Type` instead of `T` to define Types within FIMS.

#### 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](https://www.conventionalcommits.org), which is our style guide for commit messages.

### 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](#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 "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

## GitHub Collaborative Environment

Communication is managed via the [NOAA-FIMS](https://github.com/NOAA-FIMS) GitHub organization.

### GitHub Issues

[GitHub Issues](https://guides.github.com/features/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](https://github.com/NOAA-FIMS/FIMS/issues), before the code is altered. Issues are automatically tagged with the `status: triage_needed` tag and placed on the [Issue Triage Board](https://github.com/orgs/NOAA-FIMS/projects/21) 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](https://github.com/NOAA-FIMS/FIMS/issues/new/choose) to see the available types.

#### GitHub Issue Labels

[Labels](https://docs.github.com/en/issues/using-labels-and-milestones-to-track-work/managing-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.

#### GitHub Issue Templates

The templates for issue types are maintained in the [.github/ISSUE_TEMPLATE folder within the NOAA-FIMS/.github repository](https://www.github.com/NOAA-FIMS/.github/tree/main/.github/ISSUE_TEMPLATE), 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](http://www.docs.github.com/en/communities/using-templates-to-encourage-useful-issues-and-pull-requests).

#### GitHub Projects

[GitHub Projects](https://docs/github.com/en/issues/planning-and-tracking-with-projects) are used to keep track and prioritize issues. There are several [GitHub Projects for NOAA-FIMS](https://github.com/orgs/NOAA-FIMS/projects) such as the [Issue Triage Board](https://github.com/orgs/NOAA-FIMS/projects/21).

### GitHub Cloning

Any repository within NOAA-FIMS can be [cloned](https://docs.github.com/en/repositories/creating-and-managing-repositories/cloning-a-repository#cloning-a-repository) to a personal or virtual machine. But, contributors without write access will need to [fork](https://docs.github.com/en/get-started/quickstart/fork-a-repo#forking-a-repository) the repository and then clone their fork. 

### GitHub Branching

After cloning and before any work is done, ideas for potential changes to the code should be proposed in a [GitHub Issue](#github-issues). Next, after the issue is filed and before any work is done, a feature branch should be created where the work will be done.

#### Branch Naming Conventions

1. Keep it brief
1. Use a hyphen as separators

Example: **R-pkg-skeleton**

#### Branching Strategy

FIMS uses a [Scaled Trunk-Based Development](https://trunkbaseddevelopment.com/) 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](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/managing-a-branch-protection-rule). For example, the branch protection rules for the main branch of NOAA-FIMS/FIMS are described on [GitHub in the branch settings](https://github.com/NOAA-FIMS/FIMS/settings/branches).

### GitHub Pull Requests

Once development within a feature branch is complete, a [Pull Request (PR)](https://docs.github.com/en/pull-requests) 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](#pull-request-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](#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](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request-from-a-fork) for more information.

#### Pull Request Templates

The templates for PRs are maintained in the [.github/folder within the NOAA-FIMS/.github repository](https://www.github.com/NOAA-FIMS/.github/tree/main/.github), same as [NOAA-FIMS Issue templates](#github-issue-templates).

See the [NOAA-FIMS/FIMS PR template here](https://github.com/NOAA-FIMS/FIMS/blob/main/.github/PULL_REQUEST_TEMPLATE/pull_request_template.md).

#### Pull Request Review

[Code review](https://github.com/features/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](https://google.github.io/eng-practices/review/) 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](#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.

```{r, echo=FALSE}
DiagrammeR::mermaid("
        graph TB
        A[Author: Submit a pull request PR<br/><br/>- Documentation included<br/>- Tests implemented<br/>- CI checks passed]
        B[Author: Propose a reviewer<br/><br/>- Default assignee: an OST rep for a quick review]
        
        A-->B
        B-->C[Reviewer: Receive code review request]
        C --> |No assistance needed| E[Reviewer: Execute code review]
        C --> |Help wanted| D[Reviewer: Add an additional reviewer to the PR] 
        D --> E
        E --> |Changes needed| F[Reviewer: Request changes] 
        F --> G[Author: Make changes]
        G --> H[Reviewer: Approve PR]
        E --> |No changes needed| H
        H --> I[Author: Rebase and merge PR]
 
        style A text-align:left, fill:#FFF
        style B fill:#FFF
        style G fill:#FFF
        style I fill:#FFF
", height = 1000)

```

##### 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"](https://docs.github.com/en/account-and-profile/setting-up-and-managing-your-github-profile/customizing-your-profile/personalizing-your-profile#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.

##### 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](https://github.com/NOAA-FIMS/FIMS/blob/main/.github/workflows/pr-checklist.yml).

##### Reviewer Good Practices

Good reviews require good review habits. Please use the guidance found at [Conventional Comments](https://conventionalcomments.org/) for formatting/structuring your reviewer comments. Navigate to the [Perforce Blog](https://www.perforce.com/blog/qac/9-best-practices-for-code-review) for nine best practices for code review and use the [FIMS Style Guide](#style-guide) to settle any style arguments.

### GitHub Actions

FIMS uses [GitHub Actions](https://docs.GitHub.com/en/actions) to automate routine tasks such as testing, building and deploying websites, and notifying individuals. Some of the actions are built on [reusable workflows](https://docs.GitHub.com/en/actions/using-workflows/reusing-workflows) available in [{ghactions4r}](https://nmfs-fish-tools.github.io/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](https://github.com/NOAA-FIMS/FIMS/tree/main/.github/workflows) specify the setup for the GitHub Actions for the source code of FIMS.

#### 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](#testing).

#### 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](https://docs.GitHub.com/en/actions/quickstart#viewing-your-workflow-results).
- 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 .yml file may be needed.

## git

[git](https://git-scm.com) 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](#github-collaborative-environment)) is used as a central server. The use of git stops the need for saving files with file_1.txt and file_2.txt.

### git Example

1. 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 <branch-name> with the name of your feature branch (see [Branch Naming Conventions](#branch-naming-conventions)) and check it out.

```{r eval = FALSE}
$ git checkout main
$ git checkout -b <branch-name>
```

2. 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. 

```{r eval = FALSE}
$ git fetch -a
$ git rebase origin/main
```

3. While editing code, commit regularly following [commit messages](#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 `-m` argument and use a text file to make a more meaningful commit message with multiple lines.

```{r eval = FALSE}
$ git add <filename>
$ git commit -m "feat: 50 character description" "Longer description"
```

4. 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.
```{r eval = FALSE}
$ git rebase -i head~10
```

5. Push your committed changes to GitHub.
```{r eval = FALSE}
$ git push origin <branch-name>
```

1. When you are finished making changes to your feature branch, navigate to GitHub and open a [pull request](#github-pull-request) to the desired branch, e.g., main in this example.

2. 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 `-D` instead 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.

```{r, eval = FALSE}
$ git checkout main
$ git branch -d <branch-name>
```