charliemirabile
commented
2 days ago for the record, this is the text of the peer review email that is being removed
Hello everyone,
For peer review, find the row with your name in the left column
and review the patches submitted any others in that row:
-- Begin Table --
{review_table}
-- End Table --
Begin each review by creating a new branch based off the latest commit
to the master branch of upstream ILKD_submissions repository.
Your review must involve applying all the submitted patches in
sequential order and conducting the following tests:
- You must verify each patch applies cleanly
- This means no corrupt or missing patches
- You must verify that no patch adds whitespace errors i.e.:
- No whitespace at the end of any lines
- No extra blank lines at the end of a file
- Ensure there is a newline at the end of every file
- You must verify that the diffstat output right after the `---`
in each patch seems reasonable, for example:
- If this is patch 2/4, everything that the assignment directions
specify to include in patch 2 is present and nothing else
- If the directions say to put the files into a folder named after
the assignment, all files added are in such a folder
- If the directions say to add your code and a makefile, a file named
`Makefile` and at least one file with the `.c` extension are added
- No stray files unrelated to the assignment are included (e.g. code
from other assignments or `.patch` files from previous attempts
- You must verify that the actual contents of the files
added or modified by each patch are sane, for example:
- If the patch adds code, the code should compile without
errors/warnings and not immediately crash if you run it
- If the patch adds the output of a command, is there anything
actually in the file? Does it look like the kind of output
one can expect from the command?
- If the patch includes another patch, is it corrupt?
- If the patch answers provided questions, is each one answered?
Refer to the particular assignment requirements for {assignment}
and the general submission procedures on the course website for details.
Document any issues you find in detail in your reply to the cover letter
of the submission.
Your reply should end with a trailer in the style of your "Signed-off-by:"
(DCO) line, either "Acked-by:" for approval or "Peer-reviewed-by:" if
any issues are encountered.
General Tips:
- Append this line to your `.muttrc` file to wire hitting the `l` key when
in the mutt index to having mutt run `git am` in the directory where you
invoked `mutt` and piping in the currently highlighted email patch
macro index l '| git am'\\n
- This macro will not work when an email is open for viewing (unless you
add another similar line that binds the key inside of the 'pager' menu
instead of the 'index' menu)
- If patch application fails, subsequent use of this macro will fail since
git is in a "running `git am`" state. You must abort this existing
failed `git am` by running `git am --abort` in the repository.
You can do that without needing to leave mutt by pressing `!` and
entering `git am --abort` and pressing enter (The `!` shortcut
also works for running any other shell command from within mutt).
- A cover letter has no diff so it is _not_ a patch! Don't try to apply it
- An attempt to apply the cover letter will require a `git am --abort`
Having two sources of truth about how to do peer review is confusing, and now that the dashboard displays the users that the student is supposed to review, the entire peer review assignment email is kind of redundant, so I intend to remove it. However, while the peer review section of procedures.md is mostly the same as what is suggested in the peer review email, some of the details and tips in there are worth integrating into the procedures.