Of GitHub and Pull Requests (and comics)
rachelnabors.com
rachelnabors.com
As a maintainer, however, I'm far more lenient in these things. If someone takes the time to fix a bug or add a neat new feature in something I maintain, I'm greatful for it, and I'm not going to get anal about the pull request. If there's something about it I find needs changing, I'll accept the pull request and fix it myself (with a few exceptions), and inform the contributor of the changes I've made, why I made them.
I'd rather not make pull requests be an exercise in frustration for the contributor[1]. Usually, you want more people to help with your project, and frustrating your contributors isn't a good way to encourage that.
[1] I say this as someone who, with one pull request, spent several hours over a few days trying to get a 15-line change accepted (about 10 of which were comment lines), with lots of debate over the most of the requested changes being anal things like "add a space after the comma, remove a space after the =". I was damn near the end of my rope and ready to just say "fuck it, if you don't want my fix, don't take it, I'll just maintain my own fork."
I made my first contribution to a GH project a couple of weeks ago. It was only a few lines, and the maintainer had to make a few modifications before he merged it, but I got some really positive feedback from him and you could tell how grateful they were.
Months later, he silently incorporates the change...Talk about annoying..
Most of my commits are to homebrew for software I'd like in in brew but that isn't there, and also modifications to current software that is there to get it to compile on newer versions of Mac OS X.
A major maintainer of a project is generally are aware of the weak spots in the project and has a mental picture of how he would resolve them. This picture likely differs from what the pull requestor pictured, and so the implementation differs from what the maintainer imagined it would look like.
All of these small differences make the code feel foreign to the maintainer, and gives a feeling of losing control over the code. Overcoming this mental barrier is essential to being able to effectively collaborate on Github, and it's a skill that needs to be developed.
I do this because that's what people did for me when I was new. I accidentally crapped all over this SVN repo we were working on, and everyone was super professional about it - fixed all the things I'd broken, taught me how to use the tools I was using, and generally welcomed me into the group even though I had no clue what I was doing.
So yeah, send me some pull requests, even if they're broken :) ( http://github.com/jayferd )
Yes, I maintain several GitHub repos. Almost all of them are, in a sense, actively open to pull requests.
My take?
Random pull requests are fine, especially for minor tweaks and small bug fixes. Even better, if the pull request fixes a known, open bug from our Bugzilla instance. In that case, it would be good to mention the bug #. We don't have a formal style guide, but unless the code is just hideous, I probably wouldn't reject a pull request on stylistic grounds alone.
OTOH, if you come along and implement a major feature with no interaction with the existing team, things might get a little more complicated. In those case, it probably is better to establish some dialogue first, just to make sure everything fits. Likewise for fixing a major bug with lots of interactions / dependencies throughout the code base.. assuming your patch really does fix it, then yes, we'd take the pull request. But it would be good to talk through it first, walk through possible "unintended consequences," etc.
And, of course, emailing or establishing contact first is pretty much never a bad thing. Well, as long as one follows through with what they say they're going to do.
There's the implicit engineering problem right there! You've always do a bit of implementation/thinkening before even going that far. Don't want to be the Flakey Sir Promise-A-Lot, do you? But of course it is sometimes hard to draw the line if it's a reasonably interesting problem ... and that's where hearts get broken and egos get bruised.
Even better, for you and others, write down everything you can think of about the feature and what obstacles you are likely to stumble upon when implementing it.
This, incidentally, is how code reviews work at Google. The reviewer (and owner, if you are submitting code to someone else's project) only check that the change looks good. The submitter is 100% responsible for ensuring that the change is good.
Once the reviewer says the change looks good, then the submitter gets permission in the source control system to actually submit the change. That's the feature I want from Github. I want to say "looks good, please merge" and then let them have access for one commit. (I believe gerrit works this way.) Then, the next tool I'd like is hosted unit tests and results on Github, so that the submitter can immediately see that they merged their change correctly and that the project is in a good state.
Oh well. It's probably time to go back to hosting my own repositories.
This changes when you consider open source projects. Any change that is simple enough that I can scan and give my thumbs up, is likely one I can scan and click the merge button. Immediate Travis-CI results tell me everything is happy.
Anything where the merge button isn't sufficient is probably not something I'd like a stranger to push to my repository. If they're not a stranger, I'd seriously think about sharing collaboration access.
But hey, we're not necessarily done with Pull Requests. I have various ideas like this I'd like to explore at some point.
And hey, sometimes I comment & close the request with "Thanks for this patch, but this isn't something I want to support." And that's OK!
My point is that my message to the author of this comic is to be bold and keep sending patches.
hard to say without seeing the pull requests, but if you edit compiled code and call it a typo you're probably at risk of doing a lot of things wrong.
perhaps you just need to spend a little more time learning about a project before trying to push your changes. seems like all of the issues presented in the comic could have been avoided by paying more careful attention.
True - what you should do is submit a fix that takes compiled code out of the repo and replaces it with a build script.
if she was editing the wrong files, the answer is to do a little more research before diving in.
That's not always as easy as it sounds if a codebase is large, uses non-standard build environments, violates best practices (like unnecessarily checking in generated files, and, if it's really necessary to do so, not making obvious that it's a generated file -- assuming it wasn't a simple oversight on Rachel's part), ...
In my opinion, pull requests should be submitted even if you're not sure they're exactly right as long as you've done a reasonable effort to make it so:
After all, familiarizing yourself with a codebase is done by working with it, and asking for help when you're stuck -- and that's exactly what a pull request is: a request for comments, where you show others what you've done so far. Of course you could have used the mailing list instead, but if there's this nice site available where you can publish your code, which keeps track of changes and provides a commenting and message system, why not make use of it?
On few occasions I've spent more time on integrating someone else's changes than it would take me to write it myself.
Still, of course everybody learns and has to start somewhere, so I welcome pull requests.
> More importantly, to open-source authors: realize that the worst thing we can do is discourage people from submitting patches or starting and engaging in conversations about our projects.
I personally LOVE when people submit pull requests to my projects. I'm not always able to pull them in, particularly if they take the project in an undesirable direction. But I always try. Style doesn't match my "style guides"? I'll either ask you to fix the formatting, or I'll just pull your patch in and fix the formatting myself (it usually takes two seconds [3]). Documentation contains a couple inaccuracies or worded weirdly? I'll pull it in and change what needs to be changed. No tests? Ok, this one is a bit harder, but I'll either ask you to write a test, or I'll pull it in when I get time to write a corresponding test. Still, some code is better than none!
Seriously, it irritates me to no end when maintainers discourage people from contributing. The only thing that irritates me more is when people act entitled to my time to help them fix some implementation of this free software, but that's another story. People who submit pull requests of any kind almost never fall into that category.
Now, I do prefer people to open an issue before submitting a pull request, but that's only because I feel bad if they spend time coding something that is either going in the wrong direction or has already been fixed. But if you're willing to take that risk, by all means, pull away!
[2] http://www.alfajango.com/blog/communicating-with-engineers-a...
[3] In vim: `gg = G` There, formatting fixed.
A long while ago after hearing all about the virtues of open source software i decided to try and contribute to a project. I found something which needed implementing and decided to have a little go at implementing it. After a bit of discussion on the mailing list, i came up with a fairly minimal implementation which was a good starting point at least, and posted a ticket to the tracker.
For about 7 months nobody seemed to take much notice of the ticket, when all of a sudden i got a message: someone else had independently worked a new implementation, and only just noticed the ticket. In effect 99% of the work i put in was ultimately wasted.
Still was a good learning experience. Nowadays i think of open source projects as being more about people in a community rather than being merely about contributing code.
I've recently submitted a patch to the mercurial-dev list (as requested). The answer was positive but asked for tests and a better commit message.
Few days later, when I had time to fulfill the request, I found out that one of the main developers fixed the issue in a different way, with an unrelated commit message and with no tests.
Most probably he did not even notice my patch attempt on the mailing list. I guess this happens all the time in every OS project. I still found this discouraging.
Imagine working on a feature for weeks, only to realize that other people with more experience and knowledge of the project are doing the same work.
What's the incentive from the maintainer's point of view to reduce duplication of effort? The maintainer's not paying either developers, and they have a backup if one of them doesn't come thru.
But from a developer's perspective, it's frustrating to put efforts into a patch that doesn't end up being used.
The maintainer has a lot of work that needs doing, thus s/he wants to avoid duplication to get that work done. Also, s/he will know how disappointing it is to write software that is not used, and s/he will want to keep programmers happy.
Look on the bright side and consider it practice. You may not have actually contributed to those projects in the end, but surely you learned something.
Here it is in case you wonder ... https://github.com/kelvin0/PyXML2PDF