Pull Requests: Labels - Meaning of the standard labels used in CMPSC 156 for PRs
We have developed a standard set of labels that the staff use when reviewing PRs during the legacy code phase of the course.
This page is a guide to what those labels mean, and how to work with them.
Points Labels
Labels that use the color gold (#FBCA04), should be added only by the staff, and signify the number of points that a PR is worth towards the project grade for the team.
| Label | Label Text | Description | Interpretation |
|---|---|---|---|
| 5 | 5 points | A very straightforward, relatively simple change | |
| 10 | 10 points | A reasonably complex change (the normal case for most PRs) | |
| 20 | 20 points | Rarely used. Reserved for changes that show signficiant initiative or effort, typically something beyond the normal expectations for the course. |
There is also a 0 points value label (), which is sometimes used to mark:
- PRs where the target is not the main branch
- PRs that restore a previously merged PR that earned points, but was temporarily reversed for some reason.
- In some cases, for PRs that fix a bug with previously merged code (this is at the discretion of the staff; these are decided on a case-by-case basis).
FIXME labels
These are used to indicate that the staff are blocked from proceeding with this PR for one of several reasons. The actions that the team needs to take are explained in the table below.
Once the team member has addressed the concern, they should remove the label to signal to the staff to take a new look.
These labels use the color bright red, #D93F0B,
| Label | Label Text | Description | Interpretation |
|---|---|---|---|
| FIXME-PR Title | Please improve the PR Title | See guidelines for PRs titles | |
| FIXME-PR Description | Please improve the PR Description | See guidelines for PRs descriptions | |
| FIXME-Team CR | Please get a code review from a member of the team | Every PR needs an up-to-date code review from a member of the team before staff will review it. | |
| FIXME-Commented-Out-Code | PRs into main should not contain commented out code | See: Commented Out Code | |
| FIXME-Deploy | Please deploy to dokku and put link in PR description | For PRs with complex logic changes, staff may want to test the code on a dokku deployment before merging | |
| FIXME-Storybook | Please link to a storybook deployment | For PRs that impact frontend changes, please link to a storybook entry (or entries) that is available fromthe gh-pages website and hosted on chromatic.com (not localhost). Link(s) should go straight to the storybook page(s) for the components/pages changed | |
| FIXME-Screenshots | Please add a screenshot to the PR description | For PRs that change the user interface, please include screenshots in the PR description | |
| FIXME-Test-Plan | Please add a ## Test Plan section to PR description | For PRs with complex logic changes, code reviewers will need the developer to specify how to test the changes on a localhost or dokku deployment, step-by-step | |
| FIXME-Link-To-Issue | Please link to one or more issues | Use the Closes #xx keyword in the PR description | |
| FIXME-Assign-PR | Please assign the PR to one or more team members | Use the right side-bar on the PR page to assign to one or more team members. | |
| FIXME-Assign-Issue | Please assign the linked issue(s) to one or more team members | Use the right side-bar on the PR page to assign the linked issue(s) to one or more team members. | |
| FIXME-Merge Conflicts | Please resolve the merge conflicts | There are merge conflicts that need to be addressed before this PR is mergeable | |
| FIXME-Rebase-On-Main | Please rebase this PR on main | The PR may include changes that were already merged, making it difficult to code review; please rebase on main so that only the relevant changes appear | |
| FIXME-Address-CR | Please address the concerns raised in the Code Review | There are concerns raised by one or more code reviewers that need to be addressed before this can be reviewed again by staff. |
Progress Labels
Labels in bright green (#0E8A16,) indicate some kind of progress is taking place on the PR.
| Label | Label Text | Description | Interpretation |
|---|---|---|---|
| Ready-For-Staff | This is ready for the staff to review | Members of the team can use this when they beleive they have the PR ready for staff review (i.e. all criteria on this checklist have been met. Staff may also use this when doing a “first pass” to see which PRs are ready, before coming back to do a more detailed review. | |
| Merge Candidate-CI | Staff beleives this is mergeable if/when CI/CD passes | This is placed by staff only on PRs that have met all of the criteria to be mergeable (including an approved code review by staff), but, perhaps due to rebasing or other reasons, needed to be run through CI another time after the code review. If the CI/CD pipeline fails, this may be removed. | |
| Merge Candidate-Dokku | Staff has reviewed code but still needs to test a deployment | A code review was done and the code looks good to the staff, but the changes was sufficiently complex that they want to test on dokku before finally merging. This should only be added by staff; after the testing, the PR will either be merged, or if a bug is found, the approving code review will be superseded by one describing the bug, and FIXME-Address-CR will be added. | |
| Merge Candidate-HOLD | PR is mergeable but depends on another PR to be merged first | The PR description will typically spell out if a later PR depends on an earlier one. If the code in this PR is good, but a previous PR is being held up for some reason, this label may be applied by staff |