Skip to content

Instantly share code, notes, and snippets.

@ross-spencer
Last active June 9, 2019 18:16
Show Gist options
  • Save ross-spencer/f7df0ad9e327555691485648002872e6 to your computer and use it in GitHub Desktop.
Save ross-spencer/f7df0ad9e327555691485648002872e6 to your computer and use it in GitHub Desktop.
Walkthrough: Making pull-requests in the Archivematica project

Walkthrough: Making pull-requests in the Archivematica project

There are a number of steps to follow to make a new pull-request against the Archivematica project. This walk-through should help guide folk through that.

Requirements

  • A Linux-based operating system.
  • A GitHub account.
  • Git installed.
  • Git configured, e.g. user.email and user.name.
  • A read-through of the CONTRIBUTING GUIDE
  • A signed Artefactual contributor's agreement.

Summary

A pull request has the following steps:

  • A Github issue created describing the problem that needs to be solved.
  • Clone the repository you are interested in working on. (If you do not work for Artefactual this clone will be in a fork of your own.)
  • Create a branch to work against.
  • Make your changes.
  • Test your changes.
  • Commit and push your changes to GitHub.
  • Open a PR against your work and describe what you've done.
  • Code review and follow-up changes.
  • Wait for approval from the project!

Worked example using the automation-tools

  1. Open an issue in the Archivematica issues repository

This primary repository hosts all issues associated with all Archivematica projects. Further information can be found in the Archivematica forum

We ask all tickets be opened with a statement of the problem, prefixed with the work Problem: an example might be Problem: Dspace transfers are failing during package extraction

There are issue templates too, e.g. the bug template asks you to describe what the correct behavior should be. You can provide additional information here, for example, if you have tried to solve the issue, what worked?

  1. Clone the repository:

$ git clone https://github.com/artefactual/automation-tools

  1. Read the README.md:

To get an idea for the project specific things to consider when contributing. The repository might have its own CONTRIBUTING.md as well, and that will inform how you participate.

README.md: https://github.com/artefactual/automation-tools/blob/master/README.md

  1. Create a virtual environment and run tests:
$ virtualenv venv
$ activate virtualenv
$ cd automation-tools
$ source venv/bin/activate
$ pip install -r requirements.txt
$ tox

Everything should pass!

To deactivate (once you've completed your work, or to return to the standard terminal: $ deactivate

  1. Create and switch to a new branch:

$ git checkout -b dev/issue-{issue-no}-{issue-name}

An example might be:

$ git checkout -b dev/issue-1-foo-bar

The issue number should be related to a ticket in the Archivematica issues repository.

  1. Open a Python IDE and make your changes:

$ subl ../automation-tools

  1. As you progress your changes, the tests should still pass, periodically run:

$ tox

Archivematica uses the black formatter and continuous integration is configured to ensure that it is run correctly for each commit pushed to the server. If tox shows black errors then you can run it manually on your file, e.g. $ black transfers/my_changed_file.py. The code will be formatted 'in-place' and the formatting will automatically match that used across the Archivematica code-base. black does not require any additional command-line arguments. It is run using its vanilla defaults by our team.

  1. When you have finished, and if tests pass, add your changes:
$ git add transfers/my_changed_files.py
$ git add tests/test_transfers.py

NB You might not add tests for every commit, although it is good practice. for a repository like automation-tools, the units aren't split well- enough to promote this level of maintenance yet. Further, changes might just be minor and can be verified in testing.

  1. Commit your changes:

$ git commit

Example commit message: https://github.com/artefactual/archivematica-docs/wiki/Style-guide#4-commit-messages

As per the Artefactual contributing.md Work done between commits should be atomic, so you might choose to separate them based on function, e.g.


  • Commit 1: changes the start transfer function.
  • Commit 2: changes the approve transfer function.
  • Commit 3: changes the code style to improve readability.

This is to help with code-review. But also, see below. That the sum of the commits will become a single commit at the end of the process using a technique called rebase-merge.

It's also a great idea to look at a repositories commit history to study how they have been written in the past:

https://github.com/artefactual/automation-tools/commits/master

  1. Push to the server:

$ git push

The first push to the server will ask you to do the following, using the branch example above:

$ git push --set-upstream origin dev/issue-1-foo-bar

  1. Find your branch on the server and follow instructions to create your pull request (PR):
  • Goto: https://github.com/artefactual/automation-tools

  • From the branch drop-down select: dev/issue-1-foo-bar

  • A pull-request button will appear next to that

  • The pull-request works from left to right, the left branch listed is the one you want to merge into, usually: master, qa/1.x, qa/0.x

  • Fill in the details on the PR form.

  • Before submitting the PR ensure the PR is connected to an issue by writing:

    Connected to archivematica/issues#1

  • Where #1 is the number of the Github issue, e.g. #456.

  1. Submit the form, and label it Status: In Progress.

  2. Wait for continuous integration (CI) tests to complete

One important feature of Github is that it enables Artefactual projects to run continuous integration tests. You can ensure that your code runs in all environments by waiting for these tests to give you the green-light. If there are any problems, you will need to go back to your code locally and go through parts of the process, steps 5-9 again to update the code on Github until it passes.

NB. The more we can test locally the better equipped we are to mitigate this, but it might not always be possible, as is the case for some of the tests in the Archviematica project itself.

  1. Assign code-reviewers.

Someone will pick up the code-review as part of their daily work. As the code is reviewed, and changes requested, changes can be made locally, as we do through steps 5-9.

$ git push

If you ever need to keep track of what you've changed, you can list those files by using:

$ git status

and see specifically which changes using:

$ git diff

Once a review is complete, your code will be Approved and you can continue with the following steps.

  1. The conversation over code-review will continue in the Github interface.

  2. Once complete we follow a rebase-merge method:

The instructions for that are here: https://wiki.archivematica.org/Merging#Merge_process The idea behind this process is that all the changes that are made are encapsulated into a single commit, and that commit can be easily referenced by maintainers in the future.

  1. The merge will see your code available to all via one of the primary project branches, e.g. master for the automation tools. The merge instructions above also ask you to delete your branches. From there, this is done as follows:

Locally:

$ git branch -d dev/issue-1-foo-bar

On Github:

$ git push origin --delete dev/issue-1-foo-bar

  1. PR complete! Nice work! 👍

References

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment