Good manners with Git
We agreed on some best practices for handling things on Github and I thought they should be documented somewhere. This is an adaptation of practices described in the Quicksilver wiki.
If you have commits you feel should be included in the Vienna core repository, you can submit a 'Pull Request' from within your fork and branch on GitHub (see GitHub contributer workflow below for more details). Once this is done, your code changes will be checked over, tested and then merged with the main repository if no problems are found.
Here are some Pull Request Rules
- Always use Pull Request. Do not push your own commits to ViennaRSS main repository, even if you have write access to it.
- After creating a Pull Request, be patient ; do not merge it yourself immediately, even if you have write access to ViennaRSS main repository.
- It's much better to leave other committers do the merge.
- If you have write access to ViennaRSS main directory, give at least 7 days for others to comment or object before merging.
- The only exception to this rule is for increasing build reference number in order to deliver a BETA binary.
- After you merged a pull request, close or comment on related issues (if appropriate). They are often linked in the pull request itself.
- First come, first serve. If you want to work on an issue, just assign it to yourself.
- If you think you know someone who can help with an issue, just @mention the person in the comments (like "@pjrobertson don't you know something about this?"). They will be notified.
- If you have an idea on how to fix the issue but don't have the time to fix it, just leave a comment explaining your idea.
There are often problems with Git and Github on getting the correct commits into a pull request. Here is a good workflow (and a variant from it). The most important principle is to never directly merge into your own master branch, but to rebase your master branch from ViennaRSS main directory.
-
Fork on GitHub
click the big "Fork" button on https://github.com/ViennaRSS/vienna-rss - Clone you own fork
git clone git@github.com:<your_username>/vienna-rss.git - Add a remote (called ViennaRSS) linking to the main repository
cd vienna-rss
git remote add ViennaRSS git@github.com:ViennaRSS/vienna-rss.git
- Always keep the master branch clean. Never do any work/commits there.
- Instead, only pull in changes from ViennaRSS/ViennaRSS with --rebase
git pull --rebase ViennaRSS master - Make a branch from master for every separate thing you're working on (checkout with the -b option creates a new branch)
git checkout -b <branchname> master - Work...commit...work...commit...work...commit ;-)
git add .
git commit - When you finished something, push that branch to your own repository, creating a new branch on GitHub
git push origin <branchname> - Go to your repository on GitHub, select the newly created branch, then click the "Pull Request" button. Try to enter a description other developers will understand and try to mention/link any related issues.
- There will be some testing and a code review. If during this code review something comes up that should be changed, you can add more commits to a pull request, simply by pushing to the branch on GitHub (just like you did before)
- Once the pull request is accepted and merged, you can delete the branch on GitHub (note the colon in the push command) and locally
git push origin :<branchname>
git branch -D <branchname> - Pull the changes into your local master branch
git checkout master
git pull --rebase ViennaRSS master - Rebase other unpublished branches to the new master
git checkout <otherBranch>
git rebase master
=> Don't merge your own changes back into your own master branch
Having all the changes in separate branches and never merging them directly into your own master branch seems complicated, but it takes care of a couple of frequent problems:
Problem : When you do a pull request on Github, it puts all the commits in the pull request. There is no way to separate commits that don't belong together.
With our approach : since you are having only commits belonging together in any one branch, that's not a problem anymore. Each pull request can be directly merged by others into the main repository via the Github site, without any editing.
If you did already commit things that you want to separate later, you can just do a cherry-pick locally into a new branch, push that new branch to Github and do the pull request from there.
Problem : If your commits are cherry-picked by a maintainer, it creates new commits. If you merge these commits back into the branch where they came from, Github gets confused and thinks your original commits should still be merged, even though they are already in the main repository.
With our approach : since you only merge stuff from the main repository into your master branch, this also isn't a problem.
My opinion is that use of the git-up script simplifies it a bit :
- Make a branch from your master, and make sure it is up to date with the main repository
git checkout -b <branchname> master
git reup ViennaRSS master - Work...commit...work...commit...work...commit ;-)
git add .
git commit - When you finished something, make sure again that you are still up to date with the main repository
git reup ViennaRSS master - Resolve any possible conflicts, redo your tests, and again do a
git reup ViennaRSS master - push your branch to your own repository, creating a new branch on GitHub
git push origin <branchname> - Go to your repository on GitHub, select the newly created branch, then click the "Pull Request" button. Try to enter a description other developers will understand and try to mention/link any related issues.
- There will be some testing and a code review. If during this code review something comes up that should be changed, you can add more commits to a pull request, simply by pushing to your branch on GitHub (just like you did before)
- Once the pull request is accepted and merged, you may delete the branch on GitHub (note the colon in the push command) and locally
git push origin :<branchname>
git branch -D <branchname> - Pull the changes into your master branch
git checkout master
git reup ViennaRSS master
git push origin
If you, along with one other person, have decided a pull request is safe to merge to the main repository, here are the main precautions that should be taken.
- Before you complete the merge, rebuild the source and make sure everything works
- Check the list of commits on the pull requester's repository, and make sure they match those for the master branch closely
- Comment on the pull request as to why it should be merged (e.g. this is working, only a small fix, comments look good etc.)