Checklist for Contributors and Reviewers of Pull Requests

The following is a guide to be used for contributors and reviewers of jwql project 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 for 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 the jwql Style Guide?
  • 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 debugging print statements been removed?
• When applicable (e.g. in a monitoring script), does the code make use of the logging module to document its processes?
• Does the code make use of the config.json file to hide any sensitive data? See the wiki for instructions.
• Does the code contain sufficient exception handling?
• Does the code contain no deprecation warnings?
• Does the code include all necessary additions/changes to jwql.readthedocs.io (via docs/source/*.rst files)? See the wiki for instructions.
• Does the code include all necessary new dependencies or dependency updates?
  • Update requirements.txt
  • Update environment_python_3_8.yml, environment_python_3_9.yml
  • Update setup.py (only for any major new dependencies, and keep version requirements broad)
• Does the code include all necessary unit tests?
  • To see where unit tests may be lacking, check out a coverage report:
    • If you have already opened a PR on the spacetelescope/jwql repo, look at the Codecov report for that branch. (Accessible by clicking the link in the Codecov Bot comment on the PR.)
    • If you have not yet opened a PR on the spacetelescope/jwql repo, generate one in pytest:
      • pytest ./jwql/tests/ --self-contained-html --cov=./ --cov-report=html
      • The coverage report can be viewed by opening jwql/tests/htmlcov/index.html in a browser.
• Did you update CHANGES.rst with a summary of the code changes?

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 five (5) business days? 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?
• Is the PR no longer a work in progress?
• Does the CI build pass?

Pertaining to the code:

• Does the code conform to the jwql Style Guide?
• 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 code make use of the config.json file to hide any sensitive data?
• 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?
• Does the entire jwql test suite pass when testing locally?
• Has CHANGES.rst been updated with a summary of the changes?