Advertisement
Guest User

Untitled

a guest
Dec 5th, 2019
119
0
Never
Not a member of Pastebin yet? Sign Up, it unlocks many cool features!
text 3.52 KB | None | 0 0
  1. Don’t criticize the author. Be nice to the developer, critique the code.
  2.  
  3.  
  4. Be willing to receive feedback. Somebody else will always knows more than you. We should be here to help each other, not tear them down.
  5.  
  6.  
  7. Use terms like we, it, or something else to not look like you are pointing fingers at the author.
  8.  
  9.  
  10.  
  11.  
  12.  
  13. If a comment isn’t a blocker for you, let the author know. A suggestion is to tag your comment [suggestion]. That way the author knows your comment isn't a blocker.
  14.  
  15.  
  16. If you, as the author, are going to change something, please add a task to that comment. That way the reviewer(s) know that you plan on changing it and it’s a reminder for you
  17.  
  18.  
  19. If you, as the reviewer, see another comment you support, add a +1 comment to it. This to help indicate another reviewer(s) agrees with the comment.
  20.  
  21.  
  22. If you do have a blocking comment establish a way to let the author know that this is a blocking comment for you and won’t approve it until it is resolved. What we be helpful is determining what is a blocking comment? This could be standards(we should not be using view from native-base but react-native), the code may possible break(trying to map over an object instead of an array), etc.
  23.  
  24.  
  25. When reviewing a PR and you don’t have a blocking comment/question, please approve the PR. We are all trying to get things done and have a dependency on each other for code review.
  26.  
  27.  
  28. If you have questions about a PR and feel it’s going to be too long or if you have multiple questions. Reach out to the developer. It may be easier to have a 5 minute call rather than go back and forth in the PR.
  29.  
  30.  
  31. Set up time to review PR’s.
  32.  
  33. Each time you have a pr, should we add it to the dpx-app-dev channel?
  34.  
  35. Is it possible to set up default reviewers?
  36.  
  37.  
  38. Ask “dumb" questions. You may be thinking of something the author may not. Better to be told this works the way we are expecting than something get released that won’t work the way we expect.
  39.  
  40.  
  41. If you want another dev to approve your PR, call it out. Like in the top comments put: I would like @name to review this pr. This can be helpful when the author is touching a component that belongs to another team and wants to make sure they don’t break something. OR on the line, call out that dev and ask the question.
  42.  
  43.  
  44. Before adding reviewers to a PR, go over it yourself. That way you may catch something and fix it before somebody else. OR first issue it to a couple of devs to get their opinion before opening it up to the group.
  45.  
  46.  
  47. Call something out if you think there is going to be questions about something.
  48.  
  49.  
  50. If you think something can be better, but not sure how, ask the question on the file.
  51.  
  52.  
  53. If you are making a visual change, maybe include an image/video? That may help to see what the change is and answer questions reviewers may have or help to identify a possible issue.
  54.  
  55.  
  56. If you have a large change, suggest creating sub branches off of the main user story branch. Meaning create feature/f###/us####/description, than create branch feature/f###/NO-TICKET/sub-branch-description off of the feature/f###/us####/description branch. Then when you do a PR, PR the sub one into the main one. When it comes time for merging the feature one into develop, you can reference to the PR's that where already approved to help with that process. NOTE: I know we are usually good with breaking down our user stories to not be this complex, but it may be a good idea to have a plan, just in case.
Advertisement
Add Comment
Please, Sign In to add comment
Advertisement