Pull requests are the preferred way to share proposed changes to the VuFind code. They allow code to be discussed and refined prior to inclusion in the project. If you have not worked with them before, you can learn more in GitHub's documentation.
Anyone may and is encouraged to make and contribute to pull requests. Since VuFind is so flexible, a lot of libraries add custom functionality to their instances. We're thrilled when developers share this code back with us!
On this page we provide a few suggestions on making a good pull request. You can also learn more about how pull requests fit into VuFind project management in our Governance Document.
A well-constructed pull request has a clearly defined scope. It's great when the changes can be described with one sentence. Sometimes it's clearest to first do the ground-laying work and then provide the rest in a separate pull request.
If there are a separate but related changes with one depending on the other, they could be provided as multiple commits in one pull request.
When there's a systematic change affecting a number of files, for instance a code style update, keep it separate from other changes. This ensures that changes that affect functionality are not buried within other changes.
If the pull request is based on a JIRA ticket, prepend the ticket number to the pull request title. A good title could be used as a commit message with no or minor changes.
Examples of good commit messages that allow you to get an idea of what has been done:
And a couple of (fictional) bad ones for comparison:
These steps are provided as a broad suggestion on how to work with pull requests and keep things clear.
You may also want to run some of the automated tests beforehand to make sure they pass.
When a pull request is submitted, some automated tests are always run to ensure e.g. that the changes don't break functionality and adhere to the coding standards. These tests need to pass before the pull request can be merged.
In addition to the automated tests there will be a code review done by a real human. This review considers several aspects of the change, including the following:
You may receive a request for further information or clarification, suggestions for refinement and other constructive comments that aim to maintain and improve quality. Humans, nonetheless, are fallible, so feel free to question the comments if you don't find them reasonable.
If you've done changes requested in a review, please re-request review from the same person. This ensures that the pull request gets attention.
When everything checks out and all requirements are fulfilled, the pull request will get merged. After it has been merged, you can delete your own branch. It's a good idea to clean up obsolete branches regularly.
Note that an accepted and merged pull request may need to be revisited if it's found out that it had unforeseen consequences.
To-do lists make it easiest for other developers to see the state of your project. It also adds a progress visualizer to the PR list.
You can create to-do lists in the description of a PR using the following syntax:
- [ ] This is a to-do. - [x] This is a finished to-do. - [ ] Waiting to get done. - [x] Made to-do list
Using labels is a good way to get more eyes on your PR.
awaiting reply | This label indicates that the VuFind lead developers are waiting for feedback or contributions on this pull request. If you're PR has this label, the ball is in your court! |
breaking | Changes are significant enough to break backwards compatibility or cause problems in custom themes. |
discussion | This PR wants your opinion! |
help requested | This PR is looking for volunteers or feels ignored by the VuFind dev team. |
on hold | This PR is waiting for another PR or a change outside of our control (dependency update). Please note what the waiting condition is in a comment and/or the description. |
And feel free to label your PR with the closest category available: