.. _reviewers-guide:
*******************
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 :doc:`MDAKit Registry<mdakits>`.
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 :doc:`MDAKit Registry<mdakits>` 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. Make a post in on the `MDAKit Discussions`_
and tag *@MDAnalysis/mdakits-reviewers*.

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!)


``src_install``: a list of commands to install the Kit from the source code. This is a *list of strings*
(*AUTOMATIC CHECK*).


``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, we recognize that there might
  be testing frameworks that we do not support yet. In those cases please exercise flexibility when
  reviewing.
- **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*).

- *note* : care should be taken to to override the underlying ``mamba`` environment. For example, this should _not_ attempt to do a ``mamba env update`` call as it will overwrite the base mamba environment and one ends up with a conda environment without the base mdakit dependencies.  



*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. 


``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. 


``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).


.. _YAML format: https://yaml.org/

.. _MDAKit Discussions: https://github.com/MDAnalysis/mdanalysis/discussions/categories/mdakit-discussions 

.. _PyPI classifiers: https://pypi.org/classifiers/