Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update git usage page with advice on pull requests etc #84

Open
peterjc opened this issue May 22, 2016 · 8 comments
Open

Update git usage page with advice on pull requests etc #84

peterjc opened this issue May 22, 2016 · 8 comments

Comments

@peterjc
Copy link
Member

peterjc commented May 22, 2016

We could update http://biopython.org/wiki/GitUsage to be much shorter (refer to more external resources at GitHub or elsewhere rather than duplicate things) (update: perhaps not, see below), and concentrate on for Biopython specific topics like our development model (and link to http://biopython.org/wiki/Contributing or a new page specifically on coding styles, testing, etc).

For example, since we've tried to continue a single-branch model, most of our pull requests have been rebased and fast-forward merged (giving a linear history). To facilitate this, it can be helpful for the contributor to sometimes rebase and update their pull request (git fetch origin then ``git rebase origin/masterandgit push your_fork your_branch --force`), although often the person merging the PR will do this at the command line.

@peterjc peterjc mentioned this issue May 22, 2016
@MarkusPiotrowski
Copy link
Contributor

I often visited Git Usage because it is really a step-by-step introduction how to setup git/GitHub to participate to Biopython. Especially when I was away from programming for some time and then somebody asked for a bug-fix on MeltingTemp (huh, how do I refresh my master? How do I push to remote?). The situation may be similar with many novices/newcomers, who want to participate to Biopython for the first time. GitUsagecontains everything related to git/GitHub they need to make their first pull-request. So I'm in favor not to shorten GitUsage for these basic things.
But I also agree to include something about rebasing and squashing.

@peterjc
Copy link
Member Author

peterjc commented May 22, 2016

Thanks @MarkusPiotrowski - that's very useful feedback. If the current text is useful, then let's not cut that material.

@mdehoon
Copy link
Contributor

mdehoon commented May 31, 2016

Without the GitUsage page, I am completely lost.

@peterjc
Copy link
Member Author

peterjc commented May 31, 2016

OK, so the consensus is not to cut anything - but I think the later section "Commiting changes to main branch" would benefit from some review and updating (to cover rebasing and squashing commits).

@mgudapak
Copy link
Contributor

mgudapak commented Jun 1, 2016

I found this really nice blog post (which I used as a guide to get started, contribute and keep my fork upto date). Would it be useful to either point to it or appropriate some content from it for our github contribution page?

One thing though I am not that familiar or unable to understand is the approach @peterjc recommends up top, namely "... it can be helpful for the contributor to sometimes rebase and update their pull request (git fetch origin then git rebase origin/master and git push your_fork your_branch --force) ..." I had read from prior posts (like the onw I mention above) that it is preferred that the contributor fork, create a descriptive branch, do their thing to address a single/atomic issue on that branch, then submit a pulll request from that branch to original repo).

Is this not recommended or preferrable? Just trying to understand the benefits and difference in what @peterjc asked for. Sorry if this is an elementary question - I am new to this. Thanks!

@MarkusPiotrowski
Copy link
Contributor

What @peterjc is describing is not an alternative approach but an additional thing that will help to make your pull request easier to merge.
One thing is that your branch, when your pull request is not immediately merged, will sooner or later be 'behind' the upstream master, because other pull requests are merged to the upstream master. Eventually conflicts will arise, because something was changed in a file that you also want to change with your pull request. So it will be helpful for merging your pull request if you 'update' your branch with the current upstream master (this is git fetch origin, git rebase origin/master, and git push ... --force).
Another thing is squashing: Often, when you make a commit you realize that something is missing or you introduced an error etc. So you make another commit, and possibly another and another. Squashing is putting all this single commits into one, with only one commit message. Squashing is also done via git rebase ... and git push ....
When doing both, your pull request will consist of only one commit and the only differences between your branch and the upstream master are the changes of your pull request. This makes merging easier (and keeps the history simple).
This is a little bit 'advanced GitUsage' but I agree with @peterjc to include this in the GitUsage page.

@mgudapak
Copy link
Contributor

mgudapak commented Jun 1, 2016

Thanks @MarkusPiotrowski for explaining it and I now understand it well. Will make sure that future commits/pull requests are done in this manner.

@peterjc
Copy link
Member Author

peterjc commented Jun 1, 2016

Thanks @MarkusPiotrowski - you seem to have a good understanding of this. I tried to outline rebasing a branch on #83 in reply to @mgudapak before reading this issue update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants