-
Notifications
You must be signed in to change notification settings - Fork 26
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
add reviewersguide and update index.rst
- Loading branch information
1 parent
5c60ec7
commit 108e409
Showing
2 changed files
with
283 additions
and
2 deletions.
There are no files selected for viewing
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,267 @@ | ||
******************* | ||
Guide for reviewers | ||
******************* | ||
|
||
The MDAKit registration process aims to present a *low barrier of entry* for contributors to make their | ||
code available to the broader community via the `MDAKit registry`_. | ||
However, there is still a minimal set of :ref:`requirements <requirements>` that must be met, and the | ||
MDAKit must pass a review before it is officially included in the Registry. | ||
|
||
A manual review also allows for more experienced reviewers to offer best-practice suggestions and | ||
improvements to newer contributors. As such, your **aims as a reviewer** are to: | ||
|
||
#. Ensure the MDAKit can be *merged* (all necessary details *present* and *correct*), and | ||
#. Encourage *best practices* and *future work*. | ||
|
||
|
||
This page provides a guide for reviewing an MDAKit submission, including a brief overview of the | ||
registration process and: | ||
|
||
* what **AUTOMATIC** checks are performed, | ||
* what to **CHECK** manually (things that must be present before the kit can be registered), and | ||
* things to **RECOMMEND** (suggestions for improving the provided metadata, but that are not required). | ||
You can also see the :ref:`guide for submitters here<add-mdakit>`. | ||
|
||
.. note:: | ||
The `MDAKit registry`_ is still evolving. This guide will be updated as changes to the registration | ||
process are made. | ||
|
||
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. | ||
|
||
How does registration work? | ||
=========================== | ||
A prospective MDAKit contributor will register their Kit by creating a Pull Request (PR) to the MDAKit | ||
github repository, consisting of a single `metadata.yaml` file (or one per Kit, if multiple Kits are | ||
being added/modified). Details on what is included in the metadata file, and what to look for as a | ||
reviewer, are provided :ref:`below <metadatafile>`. | ||
|
||
Several checks will run automatically: | ||
|
||
- *gen_matrix* generates a list of all MDAKits updated by the PR (usually, this will be a single Kit) | ||
- *mdakit-ci* will install the MDAKit and run its own tests (as detailed in | ||
the `metadata.yaml` file) with the latest and/or develop versions of MDAnalysis | ||
- *readthedocs* will create a version of the MDAKit Registry website with the prospective kit added | ||
|
||
Once the kit has passed both these automatic checks **and** the manual checks below, you can approve the | ||
PR - after merging, the Kit will officially be part of the MDAKit Registry! | ||
|
||
What to look for as a reviewer | ||
============================== | ||
|
||
On the PR | ||
********* | ||
|
||
- **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 (follow the *Details* link), | ||
as they may incorrectly appear to pass. | ||
|
||
If checks are failing, you may need to help the contributor identify and fix the issue. A failure may | ||
be due to improper set up of `metadata.yaml` (see :ref:`below <metadatafile>`), or a local failure on | ||
the MDAKit’s end. Follow the *Details* of the failed check to find out more. | ||
|
||
Some other possible points of failure: | ||
|
||
- If the failure has a “The head commit for this pull_request event is not ahead of the base | ||
commit” error, tell the contributor to update the branch used in the PR so it is up-to-date | ||
with *main* (e.g. using `git rebase`) | ||
- A failure on installing dependencies may be due the entires in the Kit’s `pyproject.toml` not | ||
being properly defined | ||
|
||
|
||
- **CHECK**: Did the docs build correctly? | ||
|
||
View the page generated by `readthedocs` (by clicking *Details* next to the check) and find the | ||
MDAKit’s entry on the Registry page. Check the expected information and badges are present on both the | ||
main list and the Kit’s individual page. | ||
|
||
|
||
- **CHECK**: Is the metadata file named correctly and in the right location? | ||
|
||
The correct format is: `mdakits/<project_name>/metadata.yaml` (see `project_name`, below) | ||
|
||
|
||
At the MDAKit’s Project Home | ||
***************************** | ||
Follow the link to the project’s home provided in the metadata file and have a quick look at the MDAKit | ||
to see if it looks sensible. Much of the information provided in the metadata file should also be | ||
available here (e.g. a LICENCE file containing licence information, installation instructions, etc). | ||
Check these details match the metadata information. | ||
|
||
| | ||
.. _metadatafile: | ||
Inside the metadata file | ||
************************ | ||
|
||
The metadata file is in `YAML format`_. Each entry is described briefly below; see also the | ||
:ref:`template metadata file <template>` for more details and a demonstration of the | ||
proper formatting. In short, current metadata entries take the form of either a *string*, e.g.: | ||
|
||
.. code-block:: yaml | ||
project_name: MDAKitForCats | ||
description: | ||
A hypothetical and nonsensical MDAKit designed | ||
to be used by cats. | ||
or a *list of strings* (which may be only one item long): | ||
|
||
.. code-block:: yaml | ||
instructions: | ||
- obtain boxed MDAKit | ||
- remove MDAKit contents | ||
- sit in empty box | ||
Entries *required* for registration | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
`project_name`: the name of the Kit | ||
|
||
- **CHECK**: This is a *string* and must match the name of the directory the `metadata.yaml` file is placed in. | ||
- **RECOMMEND**: It is suggested that this name also matches the host repository name. | ||
|
||
|
||
`authors`: the ‘creators’ of the Kit | ||
|
||
- **CHECK**: This is a *list of strings*; either a list of names, or single entry with a link to an | ||
AUTHORS file at the project’s home. | ||
- **RECOMMEND**: An AUTHORS file is preferred (as this will be easier to update). | ||
|
||
|
||
`maintainers`: the individuals responsible for the Kit going forward; they will be pinged if the MDAKit | ||
is failing | ||
|
||
- **CHECK**: This is a *list of strings*. Each entry must be a github handle. | ||
- **RECOMMEND**: It’s expected the submitter will appear on this list. If the list contains individuals | ||
not obviously associated with the submission/project, ping them to check their agreement to be | ||
included. | ||
|
||
|
||
`description`: a free form description of the Kit | ||
|
||
- **CHECK**: This is a *string*. Give the Kit a quick look - is the description reasonable? | ||
- **RECOMMEND**: Suggest anything you think would be useful to add to the Kit’s description. There’s no | ||
upper limit on length, but ideally ~1-3 sentences should be sufficient. | ||
|
||
|
||
`keywords`: keywords that describe the Kit | ||
|
||
- **CHECK**: This is a *list of strings*. | ||
- **RECOMMEND**: Make any suggestions for things you think would be useful to add. See what | ||
keywords current MDAKits use for examples (note that keywords are case-insensitive when searching). | ||
|
||
|
||
`licence`: the licence that the Kit falls under | ||
|
||
- **CHECK**: This is a *string*, which must be the SPDX ID of an | ||
`OSI approved licence <https://opensource.org/licenses/>`_. It should match the licence identified | ||
on the project’s home, e.g. in a LICENCE file. | ||
|
||
|
||
`project_home`: a link to the Kit’s code | ||
|
||
- **CHECK**: This is a *string* and points to a reasonable location on a version-controlled repository | ||
e.g. GitHub, GitLab, BitBucket, etc. | ||
|
||
|
||
`documentation_home`: a link to the Kit’s documentation | ||
|
||
- **CHECK**: This is a *string* and points somewhere sensible, which could be a single file (e.g. a | ||
README), or a website. Minimal documentation is a requirement for an MDAKit: does the linked | ||
documentation detail what the code does, how to install it, and the basic usage? | ||
- **RECOMMEND**: While only basic documentation is required for registration, you can encourage the | ||
contributor to expand and improve their documentation in the future. | ||
|
||
|
||
`documentation_type`: the type (i.e. “level of detail”) of documentation | ||
|
||
- **CHECK**: This is a *string* - e.g. 'README' (a basic overview), 'API' (description of the code) or | ||
'UserGuide' (more thorough description and explanation of usage); or a combination ('API + UserGuide'). | ||
- **RECOMMEND**: It is not strictly enforced for the “type” to match the current appearance of a Kit’s | ||
docs. If you judge that it does not, see if the submitter intends to continue working on these (and | ||
encourage them to do so!) | ||
|
||
|
||
*Optional* entries | ||
~~~~~~~~~~~~~~~~~~ | ||
|
||
These metadata entries are *optional*. Encourage the submitter to include them, but don’t block merging | ||
the PR over them. Many of these are tested by the automatic CI, so do not need to be checked manually once | ||
CI is passing. | ||
|
||
`install`: a list of commands to install the latest release of the Kit. This is a *list of strings* (*AUTOMATIC CHECK*). | ||
|
||
- **RECOMMEND**: If the installation uses e.g. github or is otherwise complicated (many steps involved), | ||
encourage the contributor to make a release on conda-forge or PyPI. | ||
|
||
|
||
`src_install`: a list of commands to install the Kit from the source code. This is a *list of strings* | ||
(*AUTOMATIC CHECK*). | ||
|
||
|
||
`import_name`: the package name, used to import the Kit in Python. This is a *string* (*AUTOMATIC CHECK*). | ||
|
||
|
||
`python_requires`: range specifications for the versions of Python this Kit supports, e.g. “>=3.9”. This | ||
is a *string* (*AUTOMATIC CHECK*). | ||
|
||
|
||
`mdanalysis_requires`: range specifications for the versions of MDAnalysis this Kit supports, e.g. | ||
“>=2.0.0”. This is a *string* (*AUTOMATIC CHECK*). | ||
|
||
- **CHECK**: The automatic checks will test the upper bound provided, but not the lower bound. If | ||
provided, see if the lower bound seems reasonable - e.g. a newly-written Kit is likely to not actually | ||
work with early versions of MDAnalysis. | ||
- **RECOMMEND**: Ideally, the Kit works with the current version of MDAnalysis - if an upper bound to an | ||
old version is given, enquire why, and recommend updating the Kit to work with a current version. | ||
|
||
|
||
`run_tests`: a list of commands to run the Kit’s tests. This is a *list of strings* (*AUTOMATIC CHECK*). | ||
|
||
- *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. | ||
- **RECOMMEND**: While a MDAKit may be registered with only minimal tests, encourage the contributor | ||
to continue improving their tests in the future. | ||
|
||
|
||
`test_dependencies`: a list of commands for installing any dependencies required by the MDAKit’s tests. | ||
This is a *list of strings* (*AUTOMATIC CHECK*). | ||
|
||
|
||
`project_org`: the account under which the code is found - this may be an individual user account, or an | ||
organisation like MDAnalysis. This is a *string*. | ||
|
||
|
||
`development_status`: the development status of the MDAKit. | ||
|
||
- **CHECK**: This is a *string* and should match one of the `PyPI classifiers`_. | ||
- **RECOMMEND**: If you don’t think the provided status matches the actual state of the Kit’s code, you | ||
can query this - but don’t let it be a blocker. | ||
|
||
|
||
`publications`: list of publications to be cited when using this MDAKit. | ||
|
||
- **CHECK**: This is a *list of strings*, and should include any relevant publications for the Kit | ||
itself as well as key upstream publications (e.g. if the Kit heavily relies on another package with an | ||
associated publication). | ||
|
||
|
||
`changelog`: a link to the MDAKit’s changelog. | ||
|
||
- **CHECK**: This is a *string*. If included, check it points to a sensible place (e.g. a CHANGELOG | ||
file). | ||
|
||
|
||
.. _`MDAKit registry`: https://mdakits.mdanalysis.org/mdakits.html | ||
|
||
.. _YAML format: https://yaml.org/ | ||
|
||
.. _PyPI classifiers: https://pypi.org/classifiers/ |