70 comments

[ 3.1 ms ] story [ 142 ms ] thread
Link to your GH profile?

I've written dozens of pull requests and most of them either get merged, or I get a concise explaination about why they're not being merged.

probably 3/5 I have to go through a few iterations of hack and rebase before they go upstream. I'm fine with this.

With some more background maybe we can offer feedback about why your code isn't making it upstream?

(comment deleted)
I will pass absolutely no judgement on you because I have no idea who you are, what you do, what the quality of your code/work is, but have you considered that maybe there is actually something wrong with the pull requests?

With no references to examples if what you're talking about, it comes off as a bit of a temper tantrum. But that's just what it feels like, but I have no idea because I have nothing to look at to form an opinion.

Even if he's misguided, he's trying to contribute. His description of someone finding a problem with his code could be written by someone who is a poor coder and doesn't understand why. However, it also doesn't sound like the people who rejected his pulls were very convincing at explaining why.

I think the point of the article is that the pull requests he's made came to nothing other than rejection. Nothing about the experience led him to believe he should make pull requests in the future. Even if he's a poor coder, I think that's a poor result and is worth talking about.

I have had similar things happen to me.

https://github.com/joyent/node/pull/3710

I sit and argue my pull like crazy but it is just getting derailed :/ oh well.

Unless I'm confused, aren't you also the author of that gist?

https://gist.github.com/3444052 your names on that one.

The original gist is on the bottom of the revision stack on the right, it was by an anonymous user.
That's really too bad. For the record, I've written a couple hundred pull requests - no really! - and those that haven't been accepted have been because

  - The project is no longer maintained (I fork or rewrite better in this case)
  - I was actually doing it wrong
  - My pull didn't have tests and that was the convention (gotta respect conventions)
  - The author wished to keep my feature out for some random reason, but either gave me a hook or showed me how to override behavior in a clean way.
When the developer refuses to pull for not one of these reasons, it's usually because they don't have time. Which is fine, I totally understand that. I do that myself - I currently have 17 open PRs that I've sent and 20 from others in projects I maintain - so that's expected.

I would also say that when a PR isn't merged because the developer wants to write it himself, it's probably a stylistic issue and your implementation doesn't fit in with his vision for the project. You can't just pull in every request that comes by, otherwise the code becomes a complete shit-show and then debugging, maintaining, and extending it is a nightmare. I can personally attest to this, having committed this faux pas myself.

I always figured the unwritten rule for every developer is to always follow the existing code's style regardless of their own opinion of it.
That's not always the case, and sometimes if you don't know the language very well - in my case, Ruby - then certain patterns aren't available to you and it looks hokey without you realizing it.
Completely forgivable. This pull request[0], on the other hand, is probably more along the lines of what mirsadm was thinking of.

[0]: https://github.com/Aaronius/Stupid-Table-Plugin/commit/fbf3d...

That's the douchiest diff I've seen yet. I've seen diffs where a shitty and misconfigured IDE had reformatted half the code and the guy hadn't reviewed his diff, but "incidentally spaces to tab" is the biggest "fuck you reviewer" I can think of.
Meta: This is a problem w/ HNs layout, but using a fixed pitch line of 144 chars makes the entire page wide enough to require horizontal scrolling. Correction: for Chrome, just the comment, for IE9 the entire page. Is there a bullet list markup?
HN formatting is limited to the following: http://news.ycombinator.com/formatdoc

Because the monospaced format is intended for code, it assumes that lines will be short, so lines longer than ~85 chars get the overflow effect.

Agreed. I have a few projects now I've not had time to integrate patches for and they've waited months. I know they are using the patches so they aren't waiting on me, and no one else has complained so it isn't on my priority list. either. In the meantime, I'm submitting pull requests to github projects that I am getting paid at work to work on, some accepted, some denied.
(comment deleted)
nah I just edited a typo, but I feel the pain :)
Some projects are more receptive to pulls from authors outside the core team than others. Namely, large projects in use by thousands of people.

Most library authors truly appreciate these kinds of pull requests.

Bitching about pull requests in such a generalized way is destructive behavior and having it up-voted here surely isn't good for many of the impressionable people that read news here.

The first thing you should always do is create a ticket. There you can discuss if its worth spending time on a patch and what the best implementation could be. Pull requests that come up out of nowhere usually end up in the end of maintainers' queue.

Or you can continue calling people who maintain free and open source projects in their spare time douchebags and neckbeards.

I cam here to say this. If something is going to take you more than 15 minutes to do, it's better to coordinate it with the project maintainers first. If it takes you less than 15 minutes, you didn't invest enough to be bitter if your code isn't accepted.
Or you can do that, proposing a trivial enhancement, and have

1) The maintainer tell you "that isn't the designed behavior" (no kidding it doesn't do that now, that's why I want to enhance it!)

2) It marked it as a duplicate of the last handful of people that wanted similar features

3) The ticket closed

I'm looking at you, jquery-ui

ad.1: "Every program attempts to expand until it can read mail." :) The argument that the dev doesn't want particular functionality can be perfectly reasonable, and might be a sign that he's even a good one and consciously fights to avoid software bloat...
Or you can be like OpenLayers and just ignore a ticket with almost a dozen comments and two working patches and a pull request!

Sometimes people just don't want your code.

Or you can be like Zend Framework and just ignore a ticket with a working patch and then months later have someone else to win a tee-shirt because that someone who has write access to the repository just participate to a bug hunt. Sometimes people want your code. They just don't want you.
Personally, I think ticket description is more important than code, so I always create a ticket first. Unfortunately, github's issue tracker doesn't allow attachment, and thus it inadvertently promotes this "pull requests from out of nowhere" behavior.

Call me old fashioned, but I still like ticket + attached patches way more than anything else. Recently, I stumbled upon a Google Code project with several defect issues. One guy commented on each of the issue with a link to his github fork which he has applied the fixes. I clicked the link, and found that the fork has been deleted. What a waste.

> github's issue tracker doesn't allow attachment, and thus it inadvertently promotes this "pull requests from out of nowhere" behavior.

"Refs #XXX" in the description (PR or any commit) ?

I've found that if it's a pull request of any significance, if you want it merged it's good to hop on IRC and talk to one of the maintainers and get their take on it. That can head off a lot of issues and save a lot of your time. Then if you still need the change, you know up front that you'll be maintaining your own fork.
I can't say I've had a similar experience on Github, or heard anything of that sort from the many developers I know who write pull requests (or even just submit issues) on Github.

I think there should be an expectation that getting a pull accepted on a huge repo like Node or Rails should be a bit hard. When code is being used by thousands of people and companies, it's important that a lot of thought and care is put into even the smallest changes.

And finally, there is an easy way to end the negativity: respond to it by doing what they say. If they want you to do it in a different way, make a separate pull request and leave it up to the maintainers to choose the best one. Best never to argue on Github. If there's a disagreement, answer it with code.

I understand where the OP is coming from. This certainly does happen from time to time, and it's awful when it does. But I think this post is overblowing it. I certainly hope he does try sending pull requests again!

(comment deleted)
(comment deleted)
Yeah thanks for linking all of my pulls, I am not op though I just edited the gist. If you look at the bottom of the gist history you will see the original author, I just feel his/her pain
I'm with you. I added parallel test execution to PHPUnit, which its maintainers have been promising since 2005. When I started writing it, they found me and encouraged me along. So I spent two weeks on it at 16 hours a day. When I was done, they invented reasons not to merge. When I was done fixing the reasons they had invented, they said they didn't want to merge it because they were going to write it themselves.
Sounds like a pretty bad case of NIH. Maybe you should fork? (Although, I haven't heard the other side of this, so that's assuming the project's code standards were followed, your code was good quality, etc.)
Please link your fork! Heck, maybe Travis-CI will let PHP projects take advantage of it.
I find it a shame that people do that and fuck around with contributors to their project. The time comes when they need contributors to submit patches, and fewer people will bother.
PHPUnit has a very definite style to its code, perhaps the changes didn't follow the style closely enough.

Do you still have a fork? I'd love some parallel test execution.

This is highly subjective. Every project and maintainer is different.

Personally, I try to be 100% positive whenever I interact with anyone who has put their time and energy into improving a project that I maintain because I am 100% grateful for their effort. If I think that their code could be better in some way, I make that suggestion to them in a positive way. If they don't want to take my advice, then I'll merge their request and make the improvements myself. Heck, even if their code breaks something I'll still merge it in-- then fix it myself while retaining the intention of their code.

Being negative towards contributors is counter-productive.

So anyone who won't accept your pull request or doesn't think it has merit is a neckbeard living in a basement?

Yeah, this isn't totally emotion-driven or biased at all.

I'm newish to github and submitting pull requests but the OP's experience is very unfortunate and orthogonal to mine. People I don't know and who are better devs than I am have all been very nice.

I started using a small PHP framework plugin written by a guy who did everything right. I want to contribute eventually, so I asked him what his license choice will be and some questions that would hopefully keep our code in sync.

He was really polite and helpful, and wants to do an MIT-style license.

I forked his github project just to keep my pull requests and work stuff outside of that cleaner.

Not really sure what solution the OP is looking for. If your pull requests are universally rejected, perhaps you could change your approach.

Github is only the best software sharing platform anyone has devised thus far in our young discipline. You can't even message people privately, you can open a new Issue though.

I have to say I haven't had this issue. Generally I accept quality pull requests to my projects within days. Pull requests I have sent to other projects have been merged in within a couple weeks at worst...

Perhaps in the future it would be worth discussing your changes with the developer ahead of time to see if that feature is something he wants in his core product. That could save you some time and effort in the future.

I usually give constructive feedback on pull requests I can't accept and then they either drop the request or fix it. Seems to be a pretty common pattern for projects in active development.
I haven't done enough pull requests to comment on the author's experiences...but one of my only was a trivial change...almost one of personal taste...to Twitter Bootstrap and I was disproportionately excited when it got acceptd. I'm assuming there's a disproportionate letdown for the inverse situation
He describes a social problem which is very dependent on the actual maintainers, but blames it on the mechanism.

Yeah, some projects have prickly/uncooperative maintainers... so what? That's true regardless of the collaboration mechanism used, and there's no obvious reason why pull requests make this problem worse than other mechanisms.

Indeed, I would guess that pull requests make things better than many other methods, because they (1) make it much easier to review changes, (2) make it much easier for the submitter to update his changes in response to comments, (3) handily keep the discussion and patches all together in one easily accessible place, and (4) maintain a record of the request, helping to avoid the "forgot about it" problem that you get with e.g. patch requests on mailing lists ("Subject: ping" :).

Frankly, given the attitude in his rant, it seems pretty likely the problems he apparently has with maintainers aren't entirely the maintainers' fault...

I didn't read that in his post. He seemed mostly to be troubled by how easily the process can be derailed by going off topic on getting lost in details.
It doesn't seem like he's ever committed to open source before because none of this is new. So maybe Github is the problem in that it lowered the bar for this guy to put in just enough effort to get his feelings hurt.
A project I maintain has an experimental branch that takes most pull requests unless they have obvious issues (which I ask the author to fix). Only things I have not accepted so far were major GUI changes that I considered confusing. (I did accept one change that basically redid the entire GUI because it was awesome). The merges back to the master branch are several months apart, but I think it's a good idea to do deep quality control on the code AFTER it's merged to keep contributors engaged.
On the other hand, I've actually taken projects with unmerged pull requests, and pulled them into my own repo to use the new functionality/bug fixes. So they aren't necessarily just sitting there uselessly.
It might be worth commenting to that effect on the pull request. It'd at least provide a morale boost to the author of the request; and showing that someone out there is already using the changes might even make the maintainer more likely to accept them.
Sounds like a case of entitlement. Why would you waste your time working on something and not know if it is wanted or not?

It's your fault for not planning ahead.

Sounds like a case of entitlement. Why would you waste your time working on something and not know if it is wanted or not?

It's your fault for not planning ahead.

Ah the peril of general claims with no specific examples.