VuFind Documentation

This page is read only. You can view the source, but not change it. Ask your administrator if you think this is wrong.
====== Making Pull Requests ======

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 [[https://docs.github.com/en/free-pro-team@latest/github/collaborating-with-issues-and-pull-requests/about-pull-requests|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 [[https://github.com/vufind-org/vufind/blob/dev/GOVERNANCE.md|Governance Document]].

===== Contents of a Good Pull Request =====

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:

  * [VUFIND-1364] Add an 'extra' field to Params objects for custom data
  * Add explicit string cast for needle parameter in string functions
  * Replace Service Manager references with more generic Containers.

And a couple of (fictional) bad ones for comparison:

  * Fix button
  * Better improvements

===== Steps to Making the Pull Request =====

These steps are provided as a broad suggestion on how to work with pull requests and keep things clear.

  - First time only: "Fork" https://github.com/vufind-org/vufind to create your own copy of the repository.
  - Create a new branch based on the target branch (normally dev) and give it a descriptive name (e.g. dev-fix-config-comment)
  - Make the changes in this new branch, commit and push them to your repository in GitHub.
  - Go to GitHub and start a new pull request through the web interface.
  - Make sure base repository and base are correct.
  - Enter a description about what has been done and why. It's often useful to provide a short use-case when it's not obvious from the change.
  - Review the changes on this screen once more so that you can catch any inadvertent changes. Also make sure that any new files were properly added.
  - Submit the pull request.
  
You may also want to [[development:recommended_tools#meeting_project_standards|run some of the automated tests]] beforehand to make sure they pass.

==== What Happens to a Pull Request ====

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 [[development:coding_standards|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:

  * General architecture
  * Code quality, including comments
  * Security
  * Backwards-compatibility
  * Any effect the change might have on current users
  * Any new requirements or dependencies
  * Performance implications
  * Accessibility
  
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 =====

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:
<code>
 - [ ] This is a to-do.
 - [x] This is a finished to-do.
 - [ ] Waiting to get done.
 - [x] Made to-do list
</code>
{{:development:screenshot_1.png|The formatted to-do list in action.}}

===== Labels =====

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:

  * bugfix
  * improvement
  * new feature

===== Other GitHub Features =====

  * Contributor - setting this would let us know who to talk to directly if need be.
  * Milestones - The VuFind main dev team will assign these as needed.
  * Issues are located on [[http://vufind.org/jira|JIRA]].