Skip to content

Handling an Issue and Pull Requests

Bailey Harrington edited this page May 16, 2022 · 4 revisions

Please follow these steps:

1. Fork this repository into your own account

2. Decide whether the issue requires a new branch for development

  • If the issue can be handled best within an existing branch (not master)
    • Make a note in the corresponding issue that development will be carried out under that branch
  • If the issue is best handled in a new branch
    • Create a new branch with a name in the format issue_NNN (e.g. for issue #123 this would be issue_123)
    • Create this branch based from the current HEAD on master, e.g. with git checkout master && git checkout -b issue_123

3. Work on the solution within the issue branch

  • During development, it may be useful periodically to rebase to the master branch to avoid some tedious admin at the final PR/merge
git rebase master

will bring your current branch up to date with master. It can be useful to check the commit graph to see how many changes have been made on your current branch that are not in master, e.g.

% git log --graph --pretty=oneline master..                                                                                                                                               [11:04:38]
* 757126b65d083ca34485c5bbd6d2ed4c268103dc (HEAD -> fastANI, origin/fastANI) Fix minor errors in tests

It may also be useful to see all changes that differ between your branch and master, e.g.

% git log --graph --pretty=oneline master..issue_340                                                                                                                                      [10:29:18]
* 7272ff37e428bb34a3419639dad2c6d14a1ccf3f (HEAD -> issue_340, origin/issue_340) add --noextend option to CLI (related to #340)
* 80d60cb829c593ed6fb87dbe493528cd00ed62ab update CircleCI config
* 85475549c8a4e91d4ef30d901e48abceaf309730 suppress print debug statements from investigating #340
* 17128b246465fb66f8874e715ab7154ca7de50e4 fix error in ANIm coverage dataframe update
[...]

shows the commits that apply to the current branch, but not master.

4. Rebase your branch against master

  • Use git rebase master and follow the instructions to check conflicts manually, to bring your branch up to date with master prior to merging.

5. Run repository tests

  • If a new feature is added, ensure that there are tests for it, and that they integrate with pytest
  • If a bug is being fixed, ensure that the bug is covered by tests (this may require writing new tests)
  • Ensure that all tests pass locally on your machine

To run the pytest test suite, issue:

pytest -v
  • It is OK if tests XPASS and XFAIL.
  • It is OK if there are warnings that do not impact on the package function.

6. Push the branch, and submit a pull request

Use the GitHub Pull Request page to manage the pull request.

  • Complete the pull request template

    • Please link any related issues with the hash syntax (e.g. #123 for issue 123), and if this is a fix please use a phrase like fixes #123 as this allows the automated project management to pick up on issues and mark them closed.
    • Some of the checkbox tasks may not apply for all PRs; an incomplete checklist does not automatically prevent a PR from being merged, in such cases.
  • The pyani repo uses continuous integration, and automated tests will be run to check the code

    • If these do not pass, please revise the code

NB. The rest of step 6 requires access to features unavailable to users; only maintainers are expected to follow these instructions.

  • Select at least one reviewer by clicking on the Reviewers link in the sidebar
  • Select at least one assignee by clicking on the Assignees link in the sidebar
  • Assign labels to the pull request (if necessary) by clicking on the Labels link in the sidebar
  • Assign the pyani project (if necessary) by clicking on the Projects link in the sidebar
  • Assign a milestone (if necessary) by clicking on the Milestone link in the sidebar

7. If the pull request tests pass and the reviewer approves, your changes will be merged

  • Reviewers may ask for code changes/additional tests/documentation to accompany a Pull Requst as part of the review, or may add such things themselves
  • We prefer to delete the branch when a pull request is accepted
  • Those submitting Pull Requests that are merged will be acknowledged as contributors to the project