I have a policy to accept anything what is not a total crap. It is easier to edit and fix code, than educate random people. And I might remove that patch before stable release anyway. Granting commit access is different level.
[deleted]
The Art of Closing
21–26 of 26 posts
Re: The Art of Closing
#22I feel like some of these "examples" are a bit wishy-washy.
> Hi X, We really appreciate you taking the time to make this patch. However the design was not discussed prior to writing it. We do see potential in what you are trying to build, but we think it would be more effective as blah, blah, and blah. We are going to close this but would love to see you open a patch that takes the above direction. Thanks, this could really be an awesome feature!
That doesn't sound like a "no" to me. It sounds like they are tell me to change some stuff around and resubmit, which is a type of "yes, but."
Re: The Art of Closing
#23Docker is the first project I can remember using that has actually underdelivered features to the point I'm using it less and less. I would definitely not follow their model of open source development as they are an example of the opposite and equally ineffective extreme.
Docker is a really complex open source project to build. Feature wise, things need to take into account compatibility with every OS it runs on (linux/windows/bsd), all networking stacks, distributed systems, the actual image distribution project, runC etc.
Underdelivered features at that scale is to be expected — quality is a bar that can't be compromised.
Re: The Art of Closing
#24As a maintainer of dozens of fairly small OSS projects on GitHub, I learned early the importance of 'No'. Also, unmentioned here, you have to be willing to ignore the 'me too' present in a lot of PRs and feature requests; what's most important to me is 'will this new code/feature make the project easier or harder to maintain?' If harder, it had better be a stunning new addition that makes the extra maintenance worth…
Asking the author of a PR to include an automated test is a favor to the larger community - you're building momentum for this practice. As a maintainer, you shouldn't feel bad about introducing this sort of hurdle. (And if you don't have the infrastructure to run those automated tests, then it's a good excuse to get it up and running!)
> Asking the author of a PR to include an automated test is a favor to the larger community
Agreed. Once this practice is in place, it's much easier to keep going - we have 100% coverage on urllib3 and require any new contributions to meet that bar.
Re: The Art of Closing
#25It's true that there are not enough "no"s. And at the same time there are also too many "no"s. E.g., many PRs are not a PR because one wants that change in the code, but because one has a problem that needs solving and I thought about helping the developers instead of letting them do all the work. If you just close it without recognizing it in any way then the problem stays still unfixed, and motivate people to do things you won't like, like patching your project just in their company's environment and thereby creating a hidden fork.
In the end I think the question is not about "yes" or "no" but about what's the actual problem and not making the easiest choice but trying to make the best.
Re: The Art of Closing
#26"Thanks so much for spending time on this amazing patch. We really appreciate it. However I do not think this is something we want to add right now, because of yadda yadda but in the future this can change. Thanks so much!"
This tells us you are a big liar even if they didn't read your post. If a patch is amazing it should be merged, if it's not then it's obviously not amazing. This kind of message is not useful. You should be more explicit in explaining the reasons you are not accepting the patch. If you don't want the feature other contributors won't even try to write it in another way before convincing you to accept the feature. If you like the feature but didn't like the patch this should be stated if you could be kind enough to explain why you didn't like the patch, they (or others) could try other approaches.
Most of the times I get a PR I'm not interested in, my response is something like this:
"I'm not accepting this PR due to [real reason here]. Please, since I value your time and don't want it wasted, before submitting any PR, please open a ticket to discuss it first. That way I can tell you beforehand whether a change is desired or not and the reasons behind this decision and this would save you some time."
This is one of the reasons I decided to stop contributing to Rails long ago. After wasting too much with two patches that got ignored or rejected I decided contributing to Rails didn't worth my time. I often ask about the new feature before working on it. But their usual response was in the line "send us a PR and we can discuss". I feel this is plain wrong because it doesn't respect the contributor's time. They should think about the feature and decide on whether it's desired, acceptable or not. Even though they say they need to take a look at the code to know, this is not true. It's just laziness. And I'm not willing to take some time I could be practicing Mandolin to spend on some changes that have great chances of being ignored. This is a valuable wasted time.
Finally, back to the original post:
"These are just a few of the techniques we’ve used in the past. I hope if you are a maintainer of a project they are helpful for you, but I would love to know your tips as well."
It's hard to believe so, since the blog doesn't accept comments and the author does not say how he would like to get this feedback. Also this is a very selfish behavior in my opinion. I find it really frustrating when I see some idea being brought to discussion in the wide but without a comments session. Lots of what we learn comes from those discussions, sometimes even more than from the original article. When you say you would like to hear the feedback, this is selfish, because only you are going to get that feedback while other readers might be interested as well.