|
| 1 | +.. _reviewers-guide: |
| 2 | +******************* |
| 3 | +Guide for reviewers |
| 4 | +******************* |
| 5 | + |
| 6 | +The MDAKit registration process aims to present a *low barrier of entry* for contributors to make their |
| 7 | +code available to the broader community via the :doc:`MDAKit Registry<mdakits>`. |
| 8 | +However, there is still a minimal set of :ref:`requirements <requirements>` that must be met, and the |
| 9 | +MDAKit must pass a review before it is officially included in the Registry. |
| 10 | + |
| 11 | +A manual review also allows for more experienced reviewers to offer best-practice suggestions and |
| 12 | +improvements to newer contributors. As such, your **aims as a reviewer** are to: |
| 13 | + |
| 14 | +#. Ensure the MDAKit can be *merged* (all necessary details *present* and *correct*), and |
| 15 | +#. Encourage *best practices* and *future work*. |
| 16 | + |
| 17 | + |
| 18 | +This page provides a guide for reviewing an MDAKit submission, including a brief overview of the |
| 19 | +registration process and: |
| 20 | + |
| 21 | +* what **AUTOMATIC** checks are performed, |
| 22 | +* what to **CHECK** manually (things that must be present before the kit can be registered), and |
| 23 | +* things to **RECOMMEND** (suggestions for improving the provided metadata, but that are not required). |
| 24 | +You can also see the :ref:`guide for submitters here<add-mdakit>`. |
| 25 | + |
| 26 | +.. note:: |
| 27 | + The :doc:`MDAKit Registry<mdakits>` is still evolving. This guide will be updated as changes to the registration |
| 28 | + process are made. |
| 29 | + |
| 30 | +Who can be a reviewer? |
| 31 | +====================== |
| 32 | +We love for members of the community to get involved at all parts of the MDAKit process! Contact us if |
| 33 | +you’d like to be involved with reviewing MDAKit submissions. Make a post in on the `MDAKit Discussions`_ |
| 34 | +and tag *@MDAnalysis/mdakits-reviewers*. |
| 35 | + |
| 36 | +How does registration work? |
| 37 | +=========================== |
| 38 | +A prospective MDAKit contributor will register their Kit by creating a Pull Request (PR) to the MDAKit |
| 39 | +github repository, consisting of a single ``metadata.yaml`` file (or one per Kit, if multiple Kits are |
| 40 | +being added/modified). Details on what is included in the metadata file, and what to look for as a |
| 41 | +reviewer, are provided :ref:`below <metadatafile>`. |
| 42 | + |
| 43 | +Several checks will run automatically: |
| 44 | + |
| 45 | +- *gen_matrix* generates a list of all MDAKits updated by the PR (usually, this will be a single Kit) |
| 46 | +- *mdakit-ci* will install the MDAKit and run its own tests (as detailed in |
| 47 | + the ``metadata.yaml`` file) with the latest and/or develop versions of MDAnalysis |
| 48 | +- *readthedocs* will create a version of the MDAKit Registry website with the prospective kit added |
| 49 | + |
| 50 | +Once the kit has passed both these automatic checks **and** the manual checks below, you can approve the |
| 51 | +PR - after merging, the Kit will officially be part of the MDAKit Registry! |
| 52 | + |
| 53 | +What to look for as a reviewer |
| 54 | +============================== |
| 55 | + |
| 56 | +On the PR |
| 57 | +********* |
| 58 | + |
| 59 | +- **CHECK**: Are the automatic checks all passing? |
| 60 | + |
| 61 | + - You many need to manually start the checks if the contributor is new to the organisation |
| 62 | + - If the Kits’ tests use ``tox``, these need to be manually checked (follow the *Details* link), |
| 63 | + as they may incorrectly appear to pass. |
| 64 | + |
| 65 | + If checks are failing, you may need to help the contributor identify and fix the issue. A failure may |
| 66 | + be due to improper set up of ``metadata.yaml`` (see :ref:`below <metadatafile>`), or a local failure on |
| 67 | + the MDAKit’s end. Follow the *Details* of the failed check to find out more. |
| 68 | + |
| 69 | + Some other possible points of failure: |
| 70 | + |
| 71 | + - If the failure has a “The head commit for this pull_request event is not ahead of the base |
| 72 | + commit” error, tell the contributor to update the branch used in the PR so it is up-to-date |
| 73 | + with *main* (e.g. using ``git rebase``) |
| 74 | + - A failure on installing dependencies may be due the entires in the Kit’s ``pyproject.toml`` not |
| 75 | + being properly defined |
| 76 | + |
| 77 | + |
| 78 | +- **CHECK**: Did the docs build correctly? |
| 79 | + |
| 80 | + View the page generated by *readthedocs* (by clicking *Details* next to the check) and find the |
| 81 | + MDAKit’s entry on the Registry page. Check the expected information and badges are present on both the |
| 82 | + main list and the Kit’s individual page. |
| 83 | + |
| 84 | + |
| 85 | +- **CHECK**: Is the metadata file named correctly and in the right location? |
| 86 | + |
| 87 | + The correct format is: ``mdakits/<project_name>/metadata.yaml`` (see ``project_name``, below) |
| 88 | + |
| 89 | + |
| 90 | +At the MDAKit’s Project Home |
| 91 | +***************************** |
| 92 | +Follow the link to the project’s home provided in the metadata file and have a quick look at the MDAKit |
| 93 | +to see if it looks sensible. Much of the information provided in the metadata file should also be |
| 94 | +available here (e.g. a LICENCE file containing licence information, installation instructions, etc). |
| 95 | +Check these details match the metadata information. |
| 96 | + |
| 97 | + |
| 98 | +.. _metadatafile: |
| 99 | +Inside the metadata file |
| 100 | +************************ |
| 101 | + |
| 102 | +The metadata file is in `YAML format`_. Each entry is described briefly below; see also the |
| 103 | +:ref:`template metadata file <template>` for more details and a demonstration of the |
| 104 | +proper formatting. In short, current metadata entries take the form of either a *string*, e.g.: |
| 105 | + |
| 106 | +.. code-block:: yaml |
| 107 | + |
| 108 | + project_name: MDAKitForCats |
| 109 | + description: |
| 110 | + A hypothetical and nonsensical MDAKit designed |
| 111 | + to be used by cats. |
| 112 | + |
| 113 | +or a *list of strings* (which may be only one item long): |
| 114 | + |
| 115 | +.. code-block:: yaml |
| 116 | + |
| 117 | + instructions: |
| 118 | + - obtain boxed MDAKit |
| 119 | + - remove MDAKit contents |
| 120 | + - sit in empty box |
| 121 | +
|
| 122 | + |
| 123 | +Entries *required* for registration |
| 124 | +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
| 125 | + |
| 126 | +``project_name``: the name of the Kit |
| 127 | + |
| 128 | +- **CHECK**: This is a *string* and must match the name of the directory the ``metadata.yaml`` file is placed in. |
| 129 | +- **RECOMMEND**: It is suggested that this name also matches the host repository name. |
| 130 | + |
| 131 | + |
| 132 | +``authors``: the ‘creators’ of the Kit |
| 133 | + |
| 134 | +- **CHECK**: This is a *list of strings*; either a list of names, or single entry with a link to an |
| 135 | + AUTHORS file at the project’s home. |
| 136 | +- **RECOMMEND**: An AUTHORS file is preferred (as this will be easier to update). |
| 137 | + |
| 138 | + |
| 139 | +``maintainers``: the individuals responsible for the Kit going forward; they will be pinged if the MDAKit |
| 140 | +is failing |
| 141 | + |
| 142 | +- **CHECK**: This is a *list of strings*. Each entry must be a github handle. |
| 143 | +- **RECOMMEND**: It’s expected the submitter will appear on this list. If the list contains individuals |
| 144 | + not obviously associated with the submission/project, ping them to check their agreement to be |
| 145 | + included. |
| 146 | + |
| 147 | + |
| 148 | +``description``: a free form description of the Kit |
| 149 | + |
| 150 | +- **CHECK**: This is a *string*. Give the Kit a quick look - is the description reasonable? |
| 151 | +- **RECOMMEND**: Suggest anything you think would be useful to add to the Kit’s description. There’s no |
| 152 | + upper limit on length, but ideally ~1-3 sentences should be sufficient. |
| 153 | + |
| 154 | + |
| 155 | +``keywords``: keywords that describe the Kit |
| 156 | + |
| 157 | +- **CHECK**: This is a *list of strings*. |
| 158 | +- **RECOMMEND**: Make any suggestions for things you think would be useful to add. See what |
| 159 | + keywords current MDAKits use for examples (note that keywords are case-insensitive when searching). |
| 160 | + |
| 161 | + |
| 162 | +``licence``: the licence that the Kit falls under |
| 163 | + |
| 164 | +- **CHECK**: This is a *string*, which must be the SPDX ID of an |
| 165 | + `OSI approved licence <https://opensource.org/licenses/>`_. It should match the licence identified |
| 166 | + on the project’s home, e.g. in a LICENCE file. |
| 167 | + |
| 168 | + |
| 169 | +``project_home``: a link to the Kit’s code |
| 170 | + |
| 171 | +- **CHECK**: This is a *string* and points to a reasonable location on a version-controlled repository |
| 172 | + e.g. GitHub, GitLab, BitBucket, etc. |
| 173 | + |
| 174 | + |
| 175 | +``documentation_home``: a link to the Kit’s documentation |
| 176 | + |
| 177 | +- **CHECK**: This is a *string* and points somewhere sensible, which could be a single file (e.g. a |
| 178 | + README), or a website. Minimal documentation is a requirement for an MDAKit: does the linked |
| 179 | + documentation detail what the code does, how to install it, and the basic usage? |
| 180 | +- **RECOMMEND**: While only basic documentation is required for registration, you can encourage the |
| 181 | + contributor to expand and improve their documentation in the future. |
| 182 | + |
| 183 | + |
| 184 | +``documentation_type``: the type (i.e. “level of detail”) of documentation |
| 185 | + |
| 186 | +- **CHECK**: This is a *string* - e.g. 'README' (a basic overview), 'API' (description of the code) or |
| 187 | + 'UserGuide' (more thorough description and explanation of usage); or a combination ('API + UserGuide'). |
| 188 | +- **RECOMMEND**: It is not strictly enforced for the “type” to match the current appearance of a Kit’s |
| 189 | + docs. If you judge that it does not, see if the submitter intends to continue working on these (and |
| 190 | + encourage them to do so!) |
| 191 | + |
| 192 | + |
| 193 | +*Optional* entries |
| 194 | +~~~~~~~~~~~~~~~~~~ |
| 195 | + |
| 196 | +These metadata entries are *optional*. Encourage the submitter to include them, but don’t block merging |
| 197 | +the PR over them. Many of these are tested by the automatic CI, so do not need to be checked manually once |
| 198 | +CI is passing. |
| 199 | + |
| 200 | +``install``: a list of commands to install the latest release of the Kit. This is a *list of strings* (*AUTOMATIC CHECK*). |
| 201 | + |
| 202 | +- **RECOMMEND**: If the installation uses e.g. github or is otherwise complicated (many steps involved), |
| 203 | + encourage the contributor to make a release on conda-forge or PyPI. |
| 204 | + |
| 205 | + |
| 206 | +``src_install``: a list of commands to install the Kit from the source code. This is a *list of strings* |
| 207 | +(*AUTOMATIC CHECK*). |
| 208 | + |
| 209 | + |
| 210 | +``import_name``: the package name, used to import the Kit in Python. This is a *string* (*AUTOMATIC CHECK*). |
| 211 | + |
| 212 | + |
| 213 | +``python_requires``: range specifications for the versions of Python this Kit supports, e.g. “>=3.9”. This |
| 214 | +is a *string* (*AUTOMATIC CHECK*). |
| 215 | + |
| 216 | + |
| 217 | +``mdanalysis_requires``: range specifications for the versions of MDAnalysis this Kit supports, e.g. |
| 218 | +“>=2.0.0”. This is a *string* (*AUTOMATIC CHECK*). |
| 219 | + |
| 220 | +- **CHECK**: The automatic checks will test the upper bound provided, but not the lower bound. If |
| 221 | + provided, see if the lower bound seems reasonable - e.g. a newly-written Kit is likely to not actually |
| 222 | + work with early versions of MDAnalysis. |
| 223 | +- **RECOMMEND**: Ideally, the Kit works with the current version of MDAnalysis - if an upper bound to an |
| 224 | + old version is given, enquire why, and recommend updating the Kit to work with a current version. |
| 225 | + |
| 226 | + |
| 227 | +``run_tests``: a list of commands to run the Kit’s tests. This is a *list of strings* (*AUTOMATIC CHECK*). |
| 228 | + |
| 229 | +- *note*: while (minimal) tests are one of the requirements of an MDAKit, providing instructions on how to run |
| 230 | + tests in the metadata file is currently optional, in order to allow greater flexibility in |
| 231 | + what format tests take and so lower the entry barrier for new contributors. However, it is *highly |
| 232 | + recommended* here to provide this metadata. |
| 233 | +- **RECOMMEND**: While a MDAKit may be registered with only minimal tests, encourage the contributor |
| 234 | + to continue improving their tests in the future. |
| 235 | + |
| 236 | + |
| 237 | +``test_dependencies``: a list of commands for installing any dependencies required by the MDAKit’s tests. |
| 238 | +This is a *list of strings* (*AUTOMATIC CHECK*). |
| 239 | + |
| 240 | + |
| 241 | +``project_org``: the account under which the code is found - this may be an individual user account, or an |
| 242 | +organisation like MDAnalysis. This is a *string*. |
| 243 | + |
| 244 | + |
| 245 | +``development_status``: the development status of the MDAKit. |
| 246 | + |
| 247 | +- **CHECK**: This is a *string* and should match one of the `PyPI classifiers`_. |
| 248 | +- **RECOMMEND**: If you don’t think the provided status matches the actual state of the Kit’s code, you |
| 249 | + can query this - but don’t let it be a blocker. |
| 250 | + |
| 251 | + |
| 252 | +``publications``: list of publications to be cited when using this MDAKit. |
| 253 | + |
| 254 | +- **CHECK**: This is a *list of strings*, and should include any relevant publications for the Kit |
| 255 | + itself as well as key upstream publications (e.g. if the Kit heavily relies on another package with an |
| 256 | + associated publication). |
| 257 | + |
| 258 | + |
| 259 | +``changelog``: a link to the MDAKit’s changelog. |
| 260 | + |
| 261 | +- **CHECK**: This is a *string*. If included, check it points to a sensible place (e.g. a CHANGELOG |
| 262 | + file). |
| 263 | + |
| 264 | + |
| 265 | +.. _YAML format: https://yaml.org/ |
| 266 | + |
| 267 | +.. _MDAKit Discussions: https://github.com/MDAnalysis/mdanalysis/discussions/categories/mdakit-discussions |
| 268 | + |
| 269 | +.. _PyPI classifiers: https://pypi.org/classifiers/ |
0 commit comments