Peer review checklist improvements #788
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for moodledevdocs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
junpataleta
reviewed
Oct 31, 2023
There was a problem hiding this comment.
Thanks for updating this, @timhunt.
The changes look good to me, especially the updates for the accessibility section. I couldn't have worded those better. So thanks for that!
About removing (Optional) for the Accessibility check, I totally agree with you. +1000 for you to change that as well!
Once the issues from automated checks have also been addressed, I think this is good to go.
Cheers!
We should follow Moodle's own Content principles https://moodledev.io/general/contentguidelines/styleguide/voice/contentprinciples#words-and-phrases-to-avoid
Accessibility should not be optional in the peer review checklist. It is something Moodle takes seriously. On the other hand, the reasons this checklist item was made optional, was that it was written in a totally aspirational way, which was more of an accessibility test, and not a peer review. Therefore, I propose we re-word to acutally be appropriate to be part of the peer review checklist. There is already a separate link to a full Accessibility checklist. Then we can make it a normal required part of peer review.
andrewnicols
force-pushed
the
peer-checklist
branch
from
d26cceb
to
fef8bee
Compare
⚡️ Lighthouse report for the deploy preview of this PR
|
junpataleta
approved these changes
Oct 31, 2023
There was a problem hiding this comment.
Looks like the automated issues have been fixed. Thanks, @andrewnicols !
|
Sorry, I missed one: we also need #790 mergecd. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Here are three proposed changes to the Peer review checklist, as three separate commits.
It might have been a mistake to send them as a single pull request, because while I think the first two should be uncontentious, the third one might need more discussion. If you want me to separate them out, just ask (or you can do it yourself).
The three changes are:
Obviously, if this gets merged, someone will need to update the Tracker comment template.