Add guide for reviewers #119
Conversation
There was a problem hiding this comment.
This looks great, thanks @fiona-naughton !
Just putting a requested changes down because it looks like the front page no longer links to the registry anymore.
docs/source/reviewersguide.rst
Outdated
| - **CHECK**: Are the automatic checks all passing? | ||
|
|
||
| - You many need to manually start the checks if the contributor is new to the organisation | ||
| - If the Kits’ tests use `tox`, these need to be manually checked |
There was a problem hiding this comment.
Do we need to include a "why" here? I.e. that tox can make tests look like they pass even when they don't?
There was a problem hiding this comment.
sound good! I had meant to ask you about this actually, since I'm not familiar with tox myself - would a manual check in this case just mean looking through the details link rather than trusting what github says in the PR?
| - *note*: while (minimal) tests are one of the requirements of an MDAKit, providing instructions on how to run | ||
| tests in the metadata file is currently optional, in order to allow greater flexibility in | ||
| what format tests take and so lower the entry barrier for new contributors. However, it is *highly | ||
| recommended* here to provide this metadata. |
There was a problem hiding this comment.
Just to note here - whether to make tests (and installation instructions) mandatory rather than optional is currently an open issue (see #95 and #98 ) - I'll restart the discussion there, but it's likely we won't reach a consensus before this PR is merged, so I'll keep them here as optional for now.
There was a problem hiding this comment.
Agreed, let's prioritize merging this even if we don't come to a conclusion on that discussion (which seems to have stalled quite a bit).
fiona-naughton
force-pushed
the
documentation-stuff
branch
from
4394c39 to
108e409
Compare
There was a problem hiding this comment.
I'll unblock since the one thing I wanted has been addressed - that being said this guide isn't really for me. @lilyminium @orbeckst may I ask for at least one of you to have a look?
docs/source/reviewersguide.rst
Outdated
| Who can be a reviewer? | ||
| ====================== | ||
| We love for members of the community to get involved at all parts of the MDAKit process! Contact us if | ||
| you’d like to be involved with reviewing MDAKit submissions. |
There was a problem hiding this comment.
It'd probably be helpful to more specifically direct people here - not sure if we wanted to just name the usual suspects, of if we have specific MDAKits points-of-contact?
There was a problem hiding this comment.
Perhaps point to the MDAkits forum and tell them to use a team-at handle for MDAKits ... if we don't have one, we should make one.
There was a problem hiding this comment.
Nice work @fiona-naughton . I did a quick read through. Minor comments inline. Can certainly be merged and if there's anything that I overlooked we can fix iteratively.
docs/source/index.rst
Outdated
| additional resources about MDAKits and how to write and/or add your own. | ||
|
|
||
| Take me to the `MDAKit Registry`_! | ||
| ================================== |
There was a problem hiding this comment.
Is this a heading or using the style of a heading? If the latter, I'd prefer to bold-face and explicitly style and not abuse semantic markup. If it's meant as a heading then it's fine.
There was a problem hiding this comment.
I was abusing the markup in order to make it larger font-size 😅 largely since I think it helps this stand out better - there might be a better way to do that
Co-authored-by: Oliver Beckstein <[email protected]>
Co-authored-by: Oliver Beckstein <[email protected]>
|
Thank you! All lgtm... please merge!! |
Added a guide aimed at people reviewing kits (issue #103).
This is fairly bare-bones formatting at the moment – I might keep playing around with potential ways to smarten it up, but this will at least give people a chance to review the text. I’ve tried not to assume too much knowledge from the reviewer’s side, with the idea we might get people in the broader community involved.
Also played around with the contents grouping/home page a little while here, to try get more focus on the registry itself - towards #34