News for hackers. Stuff that matters.
Re:Crossroads I/O is a fork of the ZeroMQ project by psadauskas on Thursday March 15, 2012 @06:32PM (#3710944)
From the FAQ:

Why do you not accept pull requests?

Pull requests can change while being reviewed. This makes it impossible to ensure that the code being merged is the same code that has been reviewed and discussed, which compromises integrity of the codebase.

If only Git had some sort of cryptographically secure token that would uniquely identify commits, and make tampering evident.

Re:Crossroads I/O is a fork of the ZeroMQ project by mato on Thursday March 15, 2012 @07:58PM (#3711239)
It does. But to my knowledge, no one submits pull requests as a set of SHA1 hashes.
Re:Crossroads I/O is a fork of the ZeroMQ project by storborg on Thursday March 15, 2012 @08:42PM (#3711373)
What is a pull request if not an ordered list of SHA1 hashes and comments?
Re:Crossroads I/O is a fork of the ZeroMQ project by technoweenie on Thursday March 15, 2012 @08:51PM (#3711402)
GitHub pull requests are tracked by a base and a head commit. Usually, base is master, and head is some feature branch. This allows the pull request on GitHub to pull in new changes to the commit as a discussion forms around it.

So in that sense, he is correct. There's no way to ensure that clicking the merge button is indeed merging what I think it's merging. Now that I type that, we could easily just include the latest SHA in the form and check that. Problem solved!

Alternatives to this are:

* Merge manually from the command line. This is typically what we do at GitHub. * Request that people file pull requests with a SHA as the pull request head. This makes it impossible to update the pull request with future commits. * Don't use pull requests.

Re:Crossroads I/O is a fork of the ZeroMQ project by snprbob86 on Thursday March 15, 2012 @09:46PM (#3711545)
> we could easily just include the latest SHA in the form and check that

Please do! I've accidentally merged the wrong code on a few occasions.

While we're making feature requests:

* The order of commits on the "Commits" tab sometimes shuffle around, especially when a history contains a merge.

* Would be nice to let me annotate code (with comments) before "Send Pull Request". Sometimes I want to make some comments to guide a reviewer.

* Browser-push updates of comment threads!

* Let me comment on a file, not just a particular line, particularly empty ones.

* Discussions are often hard to follow via email because insufficient or misleading context is included.

* 100 other things I can't think of at the moment :-)

I love GitHub; it's critical to the way we work. Thanks!

Re:Crossroads I/O is a fork of the ZeroMQ project by nixme on Friday March 16, 2012 @12:46AM (#3712008)
Wow, this is exactly the list of improvements IMO that would make GitHub a perfect code review platform. Especially push on comment threads.
Re:Crossroads I/O is a fork of the ZeroMQ project by mato on Friday March 16, 2012 @06:09AM (#3712648)
Specifying the latest SHA as part of the merge would solve the trust problem. As other people have mentioned, allowing a final "review and fixup this patch" step before merging would also be useful for some workflows; this is what happens when you merge on the command line.

That leaves another major problem for us (the XS developers) which is that we actively encourage all development discussion and code review to be done on the mailing list where everyone involved sees it.

With GH pull requests this discussion gets fragmented into separate threads on the various pull requests.

Further, in my workflow, pulling in new changes to be committed into a pull request makes that pull request a new (version of) the original. See for example how Linux patches are discussed; you post an initial version, it gets discussed, you rework it, post a v2, and so on. At each point in time it is clear what exactly is being discussed.

Personally I have some other philosophical issues with GH that might be fun to discuss, get in touch by email if you're interested.

Re:Crossroads I/O is a fork of the ZeroMQ project by cpeterso on Friday March 16, 2012 @12:06AM (#3711910)
So GitHub pull requests point to the current head of a branch and not a specific commit? Tying a pull request to a specific commit (implicitly, not necessarily with new UI) seems like it would solve the problem.
Re:Crossroads I/O is a fork of the ZeroMQ project by psadauskas on Thursday March 15, 2012 @11:28PM (#3711815)
That was my intention, I almost always merge from the command line, except for tiny & trivial pull requests, like documentation. Its hard to run the tests through the Github web UI.
Re:Crossroads I/O is a fork of the ZeroMQ project by astrodust on Friday March 16, 2012 @03:08AM (#3712323)
It does seem crazy to merge without testing if you're following that sort of procedure.
Re:Crossroads I/O is a fork of the ZeroMQ project by technoweenie on Monday March 19, 2012 @01:40PM (#3725098)
For anyone that still cares, this has been tweaked. If you attempt to merge a pull request that has changed since you loaded the page, it'll require you to re-review the pull request.
Re:Crossroads I/O is a fork of the ZeroMQ project by storborg on Thursday March 15, 2012 @10:40PM (#3711695)
Thanks for the clarification. I, for one, think that would be a great reliability/security measure.
Re:Crossroads I/O is a fork of the ZeroMQ project by Tobu on Thursday March 15, 2012 @09:44PM (#3711538)
Most pull requests are the url of a repo and the name of a branch. Without a sha1, those aren't tamper-proof.

Another option is to make pull requests for signed tags, which build on GPG trust; or to GPG-sign a pull request email containing a sha1.

- https://lwn.net/Articles/473220/

- http://git-blame.blogspot.com/2012/01/using-signed-tag-in-pu...

Re:Crossroads I/O is a fork of the ZeroMQ project by cpeterso on Thursday March 15, 2012 @11:58PM (#3711893)
(I think psadauskas is being sarcastic.)