Skip to content

Instantly share code, notes, and snippets.

@nopolabs
Last active November 18, 2020 18:35
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 nopolabs/e95373f1892cb6ef2ca0584b18c7e479 to your computer and use it in GitHub Desktop.
Save nopolabs/e95373f1892cb6ef2ca0584b18c7e479 to your computer and use it in GitHub Desktop.
code review
Code Review
-----------
### What does this PR do?
### Why do we want to do that?
### What high level changes did you make to the code to accomplish that goal?
### What other information should the reviewer be aware of when looking at this code?
### How did you test?
[]unit []integration []dev []qa []stage
### How can we verify this is working in production?
### Non-Functional Considerations
[]Metrics []Logging []Feature Flag []Database Effects []Rate Limits
"Thumbs up" or "+1": This is great! I like this! Good job!
"Question" or "?": I have a question. Could you clarify?
"Needs Work" or "X": This must be changed. This is possibly an error or strongly violates a best practice.
"Suggestion" or "Opt": This is a suggestion, take it or leave it.
"Nit": This is a nitpick and shouldn't block approvals.
"Thought": I'm thinking out loud. I just want to share my thoughts.
"Complain": I'm complaining without an obvious solution or answer. This may or may not relate to the PR.
These context clues should prefix the comment like so:
+1: Nice job! I love this formatting!
?: Does the usage of a variable make sense here?
X: This has to change. This is a syntax error.
Opt: How about commenting this regex for clarity?
Nit: Constants are usually named with UPPER_SNAKE_CASE.
Thought: What's the status of these constants system wide? I think adding them to a new file might be good.
Complain: Bitbucket Emoji Support is lame and broken :cry:
Tip: If you can live with a change, use "Opt", otherwise use "Needs Work".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment