Feel free using the stuff below as a source of inspiration or templates or mix.
When using as templates just copy the relevant part, replace <tokens> with their definitions, remove or change as appropriate.
| character delimits alternatives.
@<username> Thank you for your submission! <issue>
<closing>
issue: Merge conflict
However, there is a conflict that prevents merger. Would you, please, resolve it?
issue: Irrelevant commits
However this PR contains commits that are out of scope of this PR. We do not want these as it may cause unnecessary conflicts that will take efforts from both you as an author and mentors. This can also can make repository history entangled.
It is most likely that this PR feature branch was spun off not from master.
Please, fix the issue. You may find this guide helpful.
closing:
If you need any clarifications or support, or have any questions or suggestions with regards to the above, please, do not hesitate asking|summoning|inquiring me.
If the PR author doesn't reflect on the comment or doesn't respond longer than 3 days:
@<username> do you need any support on the PR issue as highlighted above?
@<username> do you need any support on the code review comments above?
In a single code review iteration a code reviewer normally does the following:
- Addresses particular code fragments with either of the following:
- recognition of good solution or technique
- recognition of a technique the code reviewer learns or would borrow for his/her own use
- asking for a clarification or explanation
- suggestion to fix something
- Summarizes the code review iteration:
- approves when no fixes required
- requests for a change if any code comments requires so
- when code comments contain only (a) clarification questions or (b) recognitions, or (c) general speculations for consideration only, and (a) no changes requested or (b) it is not time for approval yet, posts a generalizing note
We also distinguish between a standalone or initial code review iteration and following code review iterations that are somehow connected with a previous iteration.
The phrases grouping below takes the above into account.
Recognition 1: It is good that you have scoped these constants to the base class.
Recognition 2: Smart trick! Didn't know. Will borrow for my own use. Thanks!
Suggestion pre-amble: My understanding of the code fragment above is that it does....
Suggestion 1: This for loop transforms array elements and puts some of them into a new array.
Please, employ #Array.map and #Array.filter instead to improve code semantics.
Suggestion 2: This variable should be renamed to something more descriptive, like secondsRemaining.
Please,
Suggestion 3: Normally functions denote actions. By convention function name should start with a verb and
also give a good idea of what's going on inside. collision is not a verb but is a noun.
Consider something like checkCollision instead.
Please, rename your function so it explains what actions happen inside.
When function returns a Boolean (true or false) a good verb to start function name is either of
is, does, check, can, has (e.g. isValid, checkValidity, containsValues, isEmpty, isNotEmpty).
Suggestion 4: The code fragment above contains a repetetive pattern.
Please, make your code DRYer.
You may find some Array
methods like .map, .forEach, .filter, .reduce helpful.
Hesitant suggestion: Can we move all these constants into a dedicated file and include it for loading before the main script? (Be ready to receive an objection or reasoning)
Inquiry for explanation: The idea behind this code fragment doesn't seem to me obvious. Can you, please, explain what you were trying to achieve and why di you opt for this particular implementation?
- Avoid obviously rhetoric questions.
- Reduce suggesting questions (e.g. "Why wouldn't we do here that and that?")
- Depersonalize your messages as much as possible (e.g. "In this code fragment you..." vs "This code fragment does...")
Recognition. 1st code review iteration: @<username> Nice|Neat|Good|Great|Perfect job! | Neatly|Well|Perfectly done!
Recognition. Nth code review iteration: @<username> You have improved the code noticeably. Well done!
Breaking bad news:
- However a couple of improvements|fixes are required yet.
- A couple of important fixes and we're done.
- Let's improve the code in a couple of iterations. Please, check comments below for suggestion on iteration one.
The phrases below can be used where it is appropriate to support healthy communication, at your discretion.
Code for humans: We write code for other humans, first of all. Semantics like good variable and function naming, constants to define mystery numbers, naming conventions in general, code formatting etc are to make things clear. Please, take care of people that will read your code, which includes not only your peers but also yourself in the future.
Explanation grounding: You explanation will help to understand your way of thinking to render a better help.
Question grounding: I need some clarification to render some better help to you. Please, respond to the question above.
No-fix warning: You are not required to fix this.
Fix postponement: Please, do not fix anything at this point. We shall see if any fix required after you explain the code fragment as per my inquiry above.
Offering help: If you need any clarification|further explanation, please, let me know.