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 of Contributors

Pertaining to the code:

• Does the code conform to the jwql Style Guide?
  • Use a linter like pylint to check for proper formatting.
• Is any of the code functionality already available via native or third-party python libraries?
• 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?
• 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.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, generate a coverage report with pytest to verify that new code is covered by tests (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.

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/master 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/master 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 Jenkins 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 in the production environment?