Checklist for Contributors and Reviewers of Pull Requests

The following is a guide to be used for contributors and reviewers of ExoCTK pull requests. Note that this document is only a guide; it should not be treated as fully comprehensive, foolproof list that must be used in all situations. Remember: a foolish consistency is the hobgoblin of little minds!

If the contributor and reviewer can answer "yes" to all of the following questions, then conceivably the proposed changes are acceptable and the PR can be merged.

Checklist of Contributors

Pertaining to the code:

• Is any of the code functionality already available via native or third-party python libraries?
• Does the code conform to PEP8 and PEP257 standards?
  • Use a linter like pylint, pydocstyle, pycodestyle, flake8, or pep8 to check for proper formatting.
• Does the code execute successfully?
  • Check that the existing test suite passes.
  • Check that any newly added functionality runs without errors.
  • If modifying the web application, check that all pages and buttons work as expected.
• Is the code documented and commented sufficiently such that it is easy to read and follow?
  • Include docstrings for all new modules, classes, and functions.
  • Include in-line comments to provide necessary context
• Have all code pertaining to debugging been removed?
• Does the code contain sufficient exception handling?
• Does the code contain no deprecation warnings?
• Are all necessary data added to the EXOCTK_DATA/ package?
• Does the code include all necessary additions/changes to exoctk.readthedocs.io (via docs/source/*.rst files)?
• Does the code include all necessary new dependencies or dependency updates?
  • Update pip_requirements.txt
  • Update env/<environment>.yml
  • Update setup.py (only for any major new dependencies, and keep version requirements broad)
• Does the code include all necessary unit tests?

Pertaining to the pull request:

• Is the PR excessively long and/or covers multiple issues? If so, consider breaking it up into multiple PRs.
• Does the PR have a concise and descriptive title?
• Does the PR link to and close the relevant issue?
  • Pro tip: Including "Closes #n" in a PR will automatically close that issue when the PR is merged.
• Does the PR have a sufficient description as to make it clear what the reasons for the changes are?
• Is the PR merging into upstream/develop from user/branchname?
• Are you listed as an assignee to the PR?
• Does the PR have proper labels?

Checklist for Reviewers

Review Timeline:

• Will you be able to review the PR within the near future? If not, please comment on the PR saying as much, so the developer knows when to expect your feedback.

Pertaining to the pull request:

• Does the PR have a concise and descriptive title?
• Does the PR have a sufficient description as to make it clear what the reasons for the changes are?
• Is the PR merging into upstream/develop from user/branchname?
• Does the PR have at least one assignee?
• Does the PR have proper labels?
• Does the Continuous Integration build pass?

Pertaining to the code:

• Does the code conform to PEP8 and PEP257 standards?
• Is the code documented and commented sufficiently such that it is easy to read and follow?
• Does the code execute successfully?
• Does the code contain sufficient exception handling?
• Does the code contain no deprecation warnings?
• Does the PR contain any necessary additions/changes to docs/source/*.rst files?
• Does the PR include any necessary new dependencies or dependency updates?
• Does the PR include any necessary unit tests?
• Does the code work as intended when testing locally?