Skip to content

Instantly share code, notes, and snippets.

@textbook
Last active January 7, 2022 11:47
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save textbook/0b390cc99e1fdeb55047e2c5cf83acde to your computer and use it in GitHub Desktop.
Save textbook/0b390cc99e1fdeb55047e2c5cf83acde to your computer and use it in GitHub Desktop.
Proposed PR template for CYF tech project repos

If I'd been writing a PR in this form for this commit, it would have looked like:


This is a:

  • New feature - new behaviour has been implemented
  • 🐛 Bug fix - existing behaviour has been made to behave
  • ♻️ Refactor - the behaviour has not changed, just the implementation
  • Test backfill - tests for existing behaviour were added but the behaviour itself hasn't changed
  • ⚙️ Chore - maintenance task, behaviour and implementation haven't changed

Description

  • Purpose - Prevents the client app from crashing when the server returns a 500 error without a body. This is a 🚑 critical hotfix to stop the app crashing, we can come back later for a proper fix to the fundamental behaviour (what should happen when there are too few words for the given configuration) later. In that context I've only tested this at the client API service level and this should only be rejected if there are major problems.
  • How to check - Go to the main page. Set the minimum word length to 1, then set the maximum word length to 1. You can see the 500 error in the network tab. On the production site this causes client to crash, but now "Something went wrong" shows on the error list instead.

Links


See example PR.

This is a:

  • New feature - new behaviour has been implemented
  • 🐛 Bug fix - existing behaviour has been made to behave
  • ♻️ Refactor - the behaviour has not changed, just the implementation
  • Test backfill - tests for existing behaviour were added but the behaviour itself hasn't changed
  • ⚙️ Chore - maintenance task, behaviour and implementation haven't changed

Description

  • Purpose -
  • How to check -

Links

Author checklist

  • I have written a title that reflects the relevant ticket
  • I have written a description that says what the PR does and how to validate it
  • I have linked to the project board ticket (and any related PRs/issues) in the Links section
  • I have added a link to this PR to the ticket
  • I have made the PR to staging from a branch named <category>/<name>, e.g. feature/edit-spaceships or bugfix/restore-oxygen
  • I have completed the manual tests described here
  • I have requested reviewers here and in my team chat channel
  • I have spoken with my PM or TL about any parts of this task that may have become out-of-scope, or any additional improvements that I now realise may benefit my project
  • I have added tests, or new tests were not required
  • I have updated any documentation (e.g. diagrams, schemas), or documentation updates were not required
  • I have updated the Swagger API documentation in src/swagger/routes/
  • I have explicitly tested the access rights for at least unauthenticated, admin and non-admin cases

Reviewer checklist

Remember that asking questions helps everyone understand why decisions have been made, so the next person can see too!

  • Pull request - read the content of the PR itself (Conversation tab) and consider the following questions:
    • Is the user story/ticket linked?
    • Does the description match the linked item?
    • Does the description tell you how to validate the work?
    • Does the build still work (Checks tab)?
    • What approach would you take to meet this goal?
  • Code changes - read through the changeset (Files changed tab) and consider the following questions:
    • Do the code changes match the description?
    • Are there any changes that should not be part of this pull request (e.g. layout changes in files not related to the functionality)?
    • Do you understand what the changes are doing, how they meet the goal of the PR?
    • Does the approach to meeting those goals seem sensible? If it's not the one you thought of, is it significantly better or worse?
    • Is this well-structured code (consistent layout, comprehensible variable names, relatively small files)?
  • Software - check out the PR branch, run the software and consider the following questions:
    • Does the product still start and run correctly?
    • Is the goal of the pull request met (i.e. new behaviour for a new feature, changed behaviour for a bug fix, identical behaviour for a refactor)?
    • In the parts of the product this PR touched:
      • Is the spelling, punctuation and grammar for user-facing text correct?
      • Does the layout/UI match the designs?
  • Project progress
    • Have new tickets been created to cover any additional actions that have appeared during this review?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment