GitHub Fork and Pull Request Workflow - Quick Reference Guide | Hacker News Reader

GitHub Fork and Pull Request Workflow - Quick Reference Guide

github.com

83 pointssusam32 comments

I just wish they would also support patches. Forcing us to fork and merge back just for GitHub is annoying. Especially for quick drive-by type patches and smaller doc changes and just minor fixes.

I git clone'd the project, built it, noticed something small, fix it locally, and then decide OK I'll just send this up real quick. oh crap. NOPE I have to do a bunch of GitHub crap with forking and merging to get it back to the master tree. Annoying.

Forking and merging is perfectly fine for regular contributions, but for smaller one-time type fixes it's just annoying.

FWIW, for smaller drive-by fixes you may press the Edit button directly on a file in some else’s repo.

And in truth, the "github crap" is clicking "Fork" and running "git remote add". Pushing and filing a PR are the same process.

Except your fork now prominently appears even after the PR is merged. However, I don't think Github has a way to see all your PRs.

Here's how to see all your PRs:

https://github.com/search?q=is:pr+author:YOURNAME&type=Issue...

Here's how to see all your open PRs:

https://github.com/search?q=is:pr+is:open+author:YOURNAME&ty...

Replace YOURNAME with your GitHub handle, or pick your colleagues names.

Nice, thanks!

Right, nothing you can do from a git client, unlike a patch.

It's a website. If you want to do it entirely from your terminal, you can use their API.

Patches require you to share the patch somewhere. Your just sharing the patch to a different interface.

Sure, but the GitHub API doesn't accept patches either, near as I can tell. You MUST create a pull request, which requires a fork: https://developer.github.com/v3/pulls/#create-a-pull-request

Unless you know something I don't?

Yes, but the built in web editor is not very nice. So you have to copy/paste it, and of course you want to test and make sure it works first, etc, etc.

I think allowing patches is just a better solution here.

There are two different methods to merge pull requests in the article. When do people prefer one over the other?

Are there any good reasons why someone should try to rebase and merge (fast-forward) a pull request to avoid a merge commit? It involves more steps than the method which creates a merge commit. What benefits warrant these additional steps?

I personally prefer merging via squash since it keeps a linear commit history without polluting the history with tons of intermediate work-in-progress commits.

A linear history is helpful when debugging since it makes git bisect a lot easier. Having individual PR commits in the history makes git bisect useless since it’s unfortunately common for individual PR commits to leave the system in a broken state.

> I personally prefer merging via squash since it keeps a linear commit history without polluting the history with tons of intermediate work-in-progress commits.

How do you deal with cases where your changes require more than a single commit? Some changes are easier to review when they're separated into several logical commits rather than having one giant diff from a single commit.

I deal with that by keeping the separate logical commits, like you said, and I squash only the commits that don't add any historical meaning (like "fix a typo" or "forgot to update the changelog"). I put the issue number in the comments so you can view the history and see a group of commits are all related to a single issue.

The squash is only performed when merging the pull request (via GitHub’s merge button on the pull request). The actual work is done and reviewed as individual commits.

The nice part of this workflow is that the changes are all squashed into a single commit in the trunk, but there’s always a link to the pull request where you can see the individual commits pre-squash.

Does the individual commits pre-squash still exist in the repo or are they only saved in the PR, ie. duplicated and saved somewhere in Github's infrastructure?

Yes, the original individual commits are kept alive and never garbage collected from GitHub's copy of the repository. You can get back to them on the website via the "Commits" tab of the PR. Local clones won't contain those branches though unless you explicitly fetch them.

You can rebase locally into commits that make sense and then merge normally on GitHub.

IMO rebasing is always worth it. Commit history is read more than it's written. For community projects, squashed focused PRs that are rebased are simplest for everyone else besides the author.

Some people like a linear history. Personally I don't, because it produces commits which have "never existed", and have certainly never been tested to check they even compile.

> Are there any good reasons why someone should try to rebase and merge (fast-forward) a pull request to avoid a merge commit?

I've always thought of it as a way to effectively create a branch, do your work, and fast-foward merge it back in to the main branch without having any other work included in it.

To put it another way, let's say you create a fork, do your work, and in the interim, several other contributors do the same thing and merge their work back into the main repo. Now, you could merge the mainline back into your fork and have a merge commit in your private branch that shows the changes that the other contributors made since you created your branch, or you could merge your branch back into the main branch at the end and have it show that it branched off earlier than the current head of the main branch.

In the first case, you now have a commit that shows changes that you didn't actually make. In the second case, it shows that you didn't base your work on the latest version of the main branch.

In contrast, if you rebase and only merge your work into the main branch via a fast-forward merge, your branch will only show commits that you made and it will effectively show that it was based on the latest version of the main branch before it was merged back in.

I have found that rebasing often creates more issues than it is worth it. Only when there are only a couple of commits I might try but you are still rewriting history: something that is destructive and needs force-pushing which I almost always disallow.

Merging does create a merge commit which is sometimes annoying (when you are merging a single-line commit, for example). It does however preserve full history.

> It does however preserve full history.

Frequently, I see a commit history like:

    Implemented feature

    Forgot semi-colon

    Fixed syntax error

    Addressing review comments

    Removed unnecessary method

History like that, other than the initial commit, is not very useful. It makes more sense to rebase to clean up the commit history such that it becomes a logical series of commits, which can then be viewed via git log or git blame.

Why would rebase require force-pushing? You can always rebase the pull request on the most current master. There would be no conflicts then and no need to force-push.

If the push fails because the master has changed during the few minutes you spent in rebasing, just rebase again on the most current master and push again. No force-push is required.

Semi-related question: Assuming I drafted up a software license with generally permissive terms (freedom to use, modify, sell, etc.), but included some perverse clause that would forbid forking (such as only allowing the software to be distributed from the canonical repository on my profile), would I have legal grounds to request repository removal from GitHub if someone pressed the repo's fork button?

Yes, when you created a GH account, you read and accepted the TOS, which enforce your project must be forkable.

Interesting, thank you!

Relevant excerpt:

> If you set your pages and repositories to be viewed publicly, you grant each User of GitHub a nonexclusive, worldwide license to use, display, and perform Your Content through the GitHub Service and to reproduce Your Content solely on GitHub as permitted through GitHub's functionality (for example, through forking).

For those interested, GitHub's entire TOS is written in a very approachable way.

https://help.github.com/articles/github-terms-of-service/

After we merge a pull request with rebase + fast forward merge, there is no merge commit created. This leads to a linear commit history.

Does anyone know anyway to look at the linear commit history alone (not GitHub) and tell where a pull request was merged and who merged it?

Git tracks the committer and the author separately: you should see the “who merged” preserved as the committer and the “who wrote it” as the author.

If each commit notes the issue number in its comment (“Add indentation and trailing white space check to CI [#123]”) then you can see easily when each merge occurred by issue number.

Additionally in GitHub there are conveniences built from these little conventions. The “#123” is displayed as a hyperlink to the issue, and merging a commit that has “Fix #123” automatically closes the issue.

In cases where a feature is big enough to justify multiple commits for clarity, you can mention “#123” in all but the last commit, and “fix #123” in the final commit. Doing so partitions the linear history.

In fact, if someone else notices and fixes a regression caused by the merge, they can push additional commits later on but mention “#123” in their commit as well for documentary purposes. Seeing downstream commits “out of sequence” but with some earlier issue number is a clue this was probably a regression.

This is true only if the merge operation leads to a new commit or altering of existing commits. But your parent comment was asking this question for merge operation where there is no new commit.

The person who merged the commit appears as the committer name only if the rebase led to a change in the SHA1 hash of the commit.

If the pull request branch is already based on master and up-to-date with master, then "git checkout master; git merge pr" performs a fast-forward merge without altering any commits. The SHA1 hash of the commits in the pull request do not change. The merger's name does not appear anywhere because the merger has not done anything except fast-forwarding master.

GitLab offers this as a merge option, and I really dislike it for this exact reason. Worse still than not knowing which merge is associated with the commit is that you can't even tell the difference between reviewed commits and those which were simply pushed directly to master.

IMO it's far preferable to have an explicit "merge" commit referencing the PR in question, whether that that is a --no-ff merge or a single linear squashed commit.

I think that’s not possible, by definition. There is no merge commit, ergo no information about the merge. You may or may not consider that a feature.

Reply on news.ycombinator.com