Skip to content

Issue Review

A.J. Stein edited this page Apr 18, 2023 · 3 revisions

Purpose

This page documents different methods and approaches to reviewing the scope of work in an issue as complete.

Overview

For an issue, the specific requirements for the issue should be the acceptance criteria. Those acceptance criteria may have generic elements as defined by the issue templates, but a community or NIST OSCAL Team member should adapt them accordingly. Those acceptance criteria aside, team members may want an additional guidance on how to review the issue for completeness. One or more team members ought to use the acceptance and this guidance if it is unclear an issue is complete.

General Guidance

Is the checklist list complete?

The checklist on a feature request or bug report is to quickly identify the longer form considerations here are met. Completed items should be checked by clicking the box or editing the checklist from [ ] to [x]. For items that are not applicable or relevant, the items should be crossed out like so ~crossed out~ and not left-as is. This notation indicates the issue driver or others are aware of the requirement, but know it is not applicable. Actively managing issue checklists reduces ambiguity.

General Website Change Guidance

Did I generate the updated website locally and test the change?

Reference Model Change Guidance

Code Changes and Code Review Guidance

For issues that require changes to the code, no issue is completely without the relevant change being merged into the relevant branch as part of a pull request. A pull request, except extreme circumstances, requires code review from at least one team member. Below are guidelines for reviewing the code review and how it pertains to completeness.

Most importantly, does it completely address the issue's goals and acceptance criteria?

When reviewing the related issue that is the cause for a code change, it is important the issue driver or assigned developers review the issue's goals and acceptance criteria. When one or more developers approve a pull request, the approvers acknowledge implicitly the acceptance criteria issue are met by the pull request. If not the issue work is not complete unless:

  • The pull request code reviews request changes and they are made to fully meet the goals and acceptance criteria.
  • The goals or acceptance criteria are amended, and the revisions section of the issue explains why and the team buys into the change.
  • Relevant work that is out of scope or too large for the sprint is broken into one or more issues for follow-on sprints. The goals or acceptance criteria for the current sprint for the current issue are adapted, and these changes are indicated in the revisions section of the issue.

Is there a style guide? Did we follow it?

Is there obsolete code or comments?

If there is obsolete code or comments

Is it testable? Are the tests passing?

Clone this wiki locally