Code Quality - department-of-veterans-affairs/caseflow GitHub Wiki

The Caseflow application has evolved rapidly since 2016. We are continually striving to improve the quality and consistency of the code in order to reduce bugs and improve our ability to deliver features. Quality code is readable code.

Tools

We use a variety of automated tools to help us in this process.

Rubocop

The .rubocop.yml file defines the Rubocop rules we adhere to. When we want to define exceptions to those rules we place special comments in the code.

Reek

Reek is a "code smell detector" that helps us identify patterns in our code we would like to avoid. The .reek.yml file allows us to define which rules and exceptions we allow.

simplecov

We rely on SimpleCov for reporting on test coverage in CircleCI. It's worth noting that SimpleCov only reports on files which are loaded while running tests; it not a static analysis tool. The main Caseflow README has a section on troubleshooting locally in a single file.

The file .simplecov stores configuration for SimpleCov, including a list of of paths which are excluded from coverage via the add_filter section. It may be prudent to periodically review this list and look for and unnecessary omissions.

The ci.rake file is used to execute coverage on CircleCI. It's important to note that, with 12 parallel runners, the coverage/index.html file in Artifacts will only show coverage encountered in those particular tests, and is thus very often an incomplete view. The circleci_verify_code_coverage task merges coverage standards from each into a coverage/combined/index.html file (usually on Parallel Run 0). This is the only one that makes sense to view, as it's the composite of coverage from the 12 workers. This file may not be written if the test run fails.

As defined in the top of ci.rake, we require 90% test coverage. Tests with lower coverage will fail CI. It is possible to exclude bits of code from simplecov by wrapping it with nocov tokens, but this is generally discouraged without discussion of the implications.

Process

We automatically run Rubocop and Reek via the Code Climate tool. Every PR is evaluated by Code Climate and must pass before being merged.

Any PR-specific exceptions to the Code Climate checks should be made as part of the PR in the .rubocop.yml, .reek.yml or Rubocop comments. We do not use the Code Climate exception/ignore/skip functionality to bypass checks.

If we do introduce new exceptions to the checks as part of the PR review process, we drop a link and brief explanation in the #appeals-code-quality Slack channel in order to increase visibility of the change.

Ultimately it is up to the PR author and reviewers to determine whether the exception is allowed.

If we see patterns in exceptions being made, discussion and decisions about more permanent changes to the rules we follow are made in the #appeals-code-quality channel.