General developer forum

I think the request peer review button only appears if the bug is assigned to you. If you are proposing a fix, it is perfectly reasonable to assign the bug to yourself.

You should leave the 'peer reviewer' field blank, unless you have a good reason to name a specific person.

The theory is that people at HQ keep an eye on bugs that are in the 'waiting for peer review' state. That used to be really terrible. It has got a bit better recently, since Dan Poltawski seems to have taken it upon himself to keep on top of reviewing.

Speaking as someone who does peer review (for quiz and question issues), the following things make the reviewer's life easier:

1. Write the testing instructions.
   • This is one less thing for the reviewer to do if the patch is good enough to submit for integration.
   • It can help the reviewer determine whether you have really understood the problem, the solution, and all the potential unintended side-effects of the change.
   • It gives the reviewer some reason to believe you have acutally tested the change yourself
   • (Speaking as a developer, I find writing the testing instructions helpful, because it is not uncommon to find that as I write them, I think of more possible bad side-effects or things I should test, and hence find problems with my fix that require it to be revised. Nice link with Moodle's Pedagogy: "we learn by expressing something for others to see.)
2. Supply the fix as a git commit, either a branch on github, or a git format-patch.
   • And please get the commit comment right.
   • The commit comment is a good place to explain the change, if it is not completely clear from the code or the comments. This is another good opportunity for you to make it clear to the reviewer that you understand the problem and the implications of your change.
3. Do not include irrelevant code clean-up in the commit.
   • If the code is a real mess, then what you can do is make one commit to fix the code formatting, but which verifiably does not make any functional change, and a separate minimal commit that actually fixes the bug. https://github.com/timhunt/moodle/compare/master...MDL-34483 is currently a good example of that.
4. Write code that is clearly correct.
   • I hope this goes without saying! When you are writing code, you are writing to communicate clearly with other developers.
   • Of course, any new code should follow the coding style (but see above about not making irrelevant changes to existing code, that make the change harder to understand).