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.
Both sides previous revisionPrevious revisionNext revision | Previous revision | ||
development:making_pull_requests [2020/12/08 12:40] – [Making Pull Requests] demiankatz | development: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! | ||
- | A few suggestions | + | On this page we provide a few suggestions |
+ | |||
+ | ===== 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' | ||
+ | |||
+ | 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 ' | ||
+ | * 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: " | ||
+ | - 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: | ||
+ | |||
+ | ==== 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: | ||
+ | |||
+ | 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, | ||
+ | |||
+ | 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