Are pull requests part of Git, or a feature of tools like GitHub, Gerrit and Atlassian Stash?
Asked Answered
H

3

65

Pull requests seem to be the common way to do code review with Git. However, it is not clear whether this term means the same when using the built-in git request-pull, or a different tool.

Are pull requests an intrinsic function of Git, or is it a common term for tools like GitHub, Gerrit or Atlassian Stash?

Is the discussion and "result" of the code review stored in the Git commit history or in a separate database?

Hanukkah answered 29/11, 2013 at 16:5 Comment(0)
D
48

Pull requests are a simple concept that originated when Git was created but has been taken to different levels since.

The essence is that you do not have push rights to the repository you want to contribute on, so instead you fork the repository, making your private copy (a clone already does this btw.) and you contribute to that one instead. And then you ask a maintainer of the original repository to pull in your changes. So you are essentially submitting a patch.

Now as I said, there are different ways to do this, but it all boils down to requesting a maintainer to pull in your changes, hence the name. The original purpose Git was created for is the Linux kernel, and they have been developing using mailing lists forever. So for them, a pull request is actually sending a patch per email; those patches are actually commit objects prepended by some normal email communication stuff—Git has tools to generate this.

git request-pull is a similar tool to generate a message asking for a pull request. But in this case, it’s closer to the pull idea. While patches can just be applied, requests created by request-pull actually tell the maintainer to pull the changes from a different remote repository.

The Git book has this example:

$ git request-pull origin/master myfork
The following changes since commit 1edee6b1d61823a2de3b09c160d7080b8d1b3a40:
  John Smith (1):
        added a new function

are available in the git repository at:

  git://githost/simplegit.git featureA

Jessica Smith (2):
      add limit to log function
      change log output to 30 from 25

 lib/simplegit.rb |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

So it’s really just a utility to generate messages to execute the underlying concept.

Other source code hosters like GitHub do this similarly as well. When you create a pull request on GitHub, you just create an issue which contains further information where the commits are the maintainer can pull. Of course, it all being on a single website, they can interlink everything a bit more and provide those one-click merge buttons for example.

But the basic concept is still the same: Requesting a maintainer to pull in some changes you have made. The only difference is the way this request is communicated with.

Dispensary answered 29/11, 2013 at 16:16 Comment(7)
Hi, I see you cite git-scm.com/book/ch5-2.html but that site has been unreachable for me for some days. Can you still reach it?Doc
@Doc That’s odd. It works for me. You could read it from source on GitHub instead though. It’s the same minus the fancy graphics.Dispensary
Thanks, that's valuable! There's evidence of any issue here too: colabti.org/irclogger/irclogger_log/git?date=2013-11-29;text=on Almost looks like some bad DNS caches to me. Somebody must have been watching, because now I can get at git-scm.com/book/ch5-2.html again too.Doc
ch5-2 is great! It reassures me what I came up with yesterday is basically right: github.com/anaran/LastScrollChrome/blob/master/contributing.mdDoc
Thank you for a great explanation! I want to implement code review in a corporate setting, so I'm trying to figure out whether to use an entirely separate system (like Atlassian's Fisheye/Crucible), a Git-based system or just having the reviewer perform the merge without extra tools. Any insight to this? And does the pull-request flow allow any commenting/discussion (and storing these) before the merge is performed?Hanukkah
Well it all boils down to how complicated you want to do it. For example, the Linux kernel does everything via patches sent over the mailing list. So essentially one sends a patch and others can discuss it, until someone decides to merge it. Other projects use Gerrit where the system manages everything and you have to adhere to its workflow quite a lot; but you get a lot control over what happens to the code, and who is responsible for what (essentially, nothing gets through without beeing seen by someone else).Dispensary
A simpler and less restrictive alternative would be to just tell every developer to work on branches and then you can openly discuss these on a mailing list, or a bug tracker, or even a meeting, before it gets merged onto the master branch. On GitHub you can do this very well by creating a pull request for another branch of the same repository. When hosting the repository on your own, you could even use something like Gitolite’s access control to restrict people to even push to the master branch directly, and only give access to a few trusted developers.Dispensary
D
11

The git request-pull command can be used to create a "pull request" in native git. When run, Git generates a summary of changes to a project and the Git Repository URL where the code can be pulled from. This can then be mailed to a mailing list or the project maintainers, who can then pull in the changes to their own repositories.

This section of the Pro Git book contains more specific information about the git request-pull command.

Doorbell answered 29/11, 2013 at 16:15 Comment(0)
C
1

The essence is that you do not have push rights to the repository you want to contribute on, so instead you fork the repository, making your private copy (a clone already does this btw.) and you contribute to that one instead.

That is no longer the only option with Git 2.29 (Q4 2020):

"git receive-pack"(man) that accepts requests by "git push" learned to outsource most of the ref updates to the new "proc-receive" hook.

See commit d6edc18, commit 1702ae6, commit c6a6a01, commit 31e8595, commit b913075, commit 63518a5, commit 195d6ea, commit 15d3af5, commit 38b9197, commit 917c612 (27 Aug 2020) by Jiang Xin (jiangxin).
(Merged by Junio C Hamano -- gitster -- in commit 6c430a6, 25 Sep 2020)

receive-pack: add new proc-receive hook

Signed-off-by: Jiang Xin

Git calls an internal execute_commands function to handle commands sent from client to git-receive-pack.

Regardless of what references the user pushes, Git creates or updates the corresponding references if the user has write-permission.

A contributor who has no write-permission, cannot push to the repository directly.
So, the contributor has to write commits to an alternate location, and sends pull request by emails or by other ways.
We call this workflow as a distributed workflow.

It would be more convenient to work in a centralized workflow like what Gerrit provided for some cases.

For example, a read-only user who cannot push to a branch directly can run the following git push(man) command to push commits to a pseudo reference (has a prefix "refs/for/", not "refs/heads/") to create a code review.

git push origin \
    HEAD:refs/for/<branch-name>/<session>  

The <branch-name> in the above example can be as simple as "master", or a more complicated branch name like "foo/bar".
The <session> in the above example command can be the local branch name of the client side, such as "my/topic".

We cannot implement a centralized workflow elegantly by using "pre-receive" + "post-receive", because Git will call the internal function "execute_commands" to create references (even the special pseudo reference) between these two hooks.
Even though we can delete the temporarily created pseudo reference via the "post-receive" hook, having a temporary reference is not safe for concurrent pushes.

So, add a filter and a new handler to support this kind of workflow.

The filter will check the prefix of the reference name, and if the command has a special reference name, the filter will turn a specific field (run_proc_receive) on for the command.
Commands with this filed turned on will be executed by a new handler (a hook named "proc-receive") instead of the internal execute_commands function.

We can use this "proc-receive" command to create pull requests or send emails for code review.

Suggested by Junio, this "proc-receive" hook reads the commands, push-options (optional), and send result using a protocol in pkt-line format.
In the following example, the letter "S" stands for "receive-pack" and letter "H" stands for the hook.

# Version and features negotiation.
S: PKT-LINE(version=1\0push-options atomic...)
S: flush-pkt
H: PKT-LINE(version=1\0push-options...)
H: flush-pkt  

# Send commands from server to the hook.
S: PKT-LINE(<old-oid> <new-oid> <ref>)
S: ... ...
S: flush-pkt
# Send push-options only if the 'push-options' feature is enabled.
S: PKT-LINE(push-option)
S: ... ...
S: flush-pkt  

# Receive result from the hook.
# OK, run this command successfully.
H: PKT-LINE(ok <ref>)
# NO, I reject it.
H: PKT-LINE(ng <ref> <reason>)
# Fall through, let 'receive-pack' to execute it.
H: PKT-LINE(ok <ref>)
H: PKT-LINE(option fall-through)
# OK, but has an alternate reference.  The alternate reference name
# and other status can be given in options
H: PKT-LINE(ok <ref>)
H: PKT-LINE(option refname <refname>)
H: PKT-LINE(option old-oid <old-oid>)
H: PKT-LINE(option new-oid <new-oid>)
H: PKT-LINE(option forced-update)
H: ... ...
H: flush-pkt  

After receiving a command, the hook will execute the command, and may create/update different reference.

For example, a command for a pseudo reference "refs/for/master/topic" may create/update different reference such as "refs/pull/123/head".
The alternate reference name and other status are given in option lines.

The list of commands returned from "proc-receive" will replace the relevant commands that are sent from user to "receive-pack", and "receive-pack" will continue to run the "execute_commands" function and other routines.

Finally, the result of the execution of these commands will be reported to end user.

The reporting function from "receive-pack" to "send-pack" will be extended in latter commit just like what the "proc-receive" hook reports to "receive-pack".

Collum answered 26/9, 2020 at 12:0 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.