Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

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.



I have no clue where that commit is coming from. How do I get to it? Thanks.


It's the same commit you linked to, but ignoring whitespace changes (&w=1, like diff -w).


Apparently the parameter w=1 fixes the white space for the diff in the browser. Is there a way to make the pull request reference that?


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.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: