About Features Downloads Getting Started Documentation Events Support GitHub

Love VuFind®? Consider becoming a financial supporter. Your support helps build a better VuFind®!

Site Tools


Warning: This page has not been updated in over over a year and may be outdated or deprecated.
development:making_pull_requests

Differences

This shows you the differences between two versions of the page.

Link to this comparison view

Both sides previous revisionPrevious revision
Next revision
Previous revision
development:making_pull_requests [2020/12/08 12:40] – [Making Pull Requests] demiankatzdevelopment:making_pull_requests [2022/04/11 16:16] (current) – [What Happens to a Pull Request] emaijala
Line 5: Line 5:
 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! 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!
  
-few suggestions to making a good pull request.+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 =====
Line 44: Line 103:
  
 ---- struct data ---- ---- struct data ----
 +properties.Page Owner : 
 ---- ----
  
development/making_pull_requests.1607431218.txt.gz · Last modified: 2020/12/08 12:40 by demiankatz