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

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

6.2 Issue Templates

There are two issue templates currently available in the FIMS repository. 1. Feature requests: this template should be used to request new features or changes to features, such that the described functionality differs from what is currently in the development plan. 2. Bug reports: this should be used to file bugs, which describe when the existing functionality differs from what is described in the development plan.

6.3 Issue Lifecycle

FIMS development will adhere to a lifecycle for issues that makes it clear which issues can be resolved when.

  • Creation — The event that marks the creation of an issue. An issue is not Active when it is Created. Issues that are opened are assigned to the FIMS Project Lead with the label: needs-triage. A issue is not considered Active until this label is removed.
  • Activation — When the needs-triage label is removed and the issue is assigned to a developer, the issue becomes Active. This event happens once in the lifecycle of an issue. Activation usually is not undone but it can be undone if an issue needs additional discussion; in this case, the needs-triage label is applied again. An issue is Active from the time it is Activated until reaches Resolution.
  • Response — This event only happens if the triage team deems an issue to a wont-fix or delayed. This requires communication with the party who opened the issue as to why this will not be addressed or will be moved to a later milestone.
  • Resolution — The event that marks the resolution of an issue. This event happens once in the lifetime of an issue. This event can be undone if an issue transitions from a resolved status to an unresolved status, in which case the system considers the issue as never had been resolved. A resolution involves a code check-in and pull request, at which point someone must review and approve the pull request before the issue can transition states.
  • In Review - The issue is “in review” after a code solution has been proposed and is being considered via a pull request. If this is approved, the issue can move into the “Closed” state.
  • Closure—The event that marks the closure of an Issue. This even happens once in the lifetime of an issue. The issue can enter the Closed state from either the “In Review” or “Response” state.
Flow chart that describes above process visually, e.g., how an issue moves from creation, to activation, to response or resolution, and is finally closed.

Figure 6.1: Flow chart that describes above process visually, e.g. how an issue moves from creation, to activation, to response or resolution, and is finally closed.

6.4 Pull Requests

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

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

6.5.1 Assigning Reviewers

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

  1. A specific member of the FIMS Development Team is requested to review a pull request, based on their specific expertise.
  2. 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.

6.5.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 Chapter 6.

6.5.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. Reviewers should evaluate the code critically and provide comment/feedback on the following items:

Readability

  • Is the code easy to understand?
  • Are there any parts of the code that are confusing?
  • Is the data flow easy to understand?
  • Is there any code commented out?
  • Does the code include any unclear names?
  • Does the code include any errors, repeats, or incomplete sections?

Functionality

  • Does the code function as it is expected to?
  • How will the change impact other parts of the system?
  • Are there any unhandled edge cases?
  • Are there other code improvements possible?

Design

  • Are files organized intuitively?
  • Are components divided up in a sensible way?
  • Does the review include too many changes? Would the code change better be broken into more focused parts?
  • Will the change be easy to maintain?
  • Does the code follow object-oriented design principles?
  • Is the code in the proper location?

Security

  • Does using this code open the software to possible security violations or vulnerabilities?
  • Is the correct encryption used?

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?

Documentation

  • Are there comments available to explain the code?
  • Is the README file complete and current? Does it adequately describe the project/changes?

Testing

  • Is the code testable?
  • Is the automated testing adequate?
  • Have dependencies been appropriately tested?
  • Does automated testing cover the code exchange adequately?
  • Could the test structure be improved?

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

6.6 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”)