As 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 it... And often, it had better include an automated test!
Oh yeah, and even in regular work too. I've noticed a lot of peer pressure in many different environments, sometimes team members feel obligated to provide a "me too" or "thumbs up" or whatever, out of fear of being the nail that sticks up and gets hammered back down.
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.
I imagine a similar rationale is used by big vendors to select features for products like Windows, Android, Safari, Outlook... The business proposition is sound, but to the user "Microsoft just never learns", or "Apple need to start bringing their hardware quality to their software", and so on.
An anecdote [1] about "No is temporary, Yes is forever":
4 months ago: However, please read the notes in Roadmap and new features before continuing with this work, as I'm not yet sure whether or not they are within the scope of Umbrella
[...]
4 months ago: I don't think it's a good idea to change it, so only Strings will be available for after, before, etc.
3 months ago: I opened it to implement what was suggested here.
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.
The pain of having a patch rejected is because of the asymmetry. No is _easy_, the submitter often put hours of work into understanding the codebase, the architecture the style of the code, figure out how to fix a bug or add a feature and then to have the response be, "No", feels like a pretty harsh rejection.
How we structure work on these projects needs to be rethought so that the majority of these don't happen. The attitude of code-or-gtfo is kinda broken with respect to how much work people put into a project with an uncertain outcome.
Maybe we should
* submit an issue outlining new feature, or technique to fix bug
* create a branch, reference that issue
* update issue for a branch review, get greenlight
* do possibly hours worth of work
* submit PR that isn't outright rejected
Just communicating through pull requests seems very macho and wasteful to me.
> No is _easy_, the submitter often put hours of work into understanding the codebase [...]
And maintainers will put many, many more hours into maintaining it. The submitter only has to interact with us once. We have to interact with their code for a long time.
> How we structure work on these projects needs to be rethought [...]
Most projects work the way you described (including Docker). You open an issue (and/or send a mail to the mailing list) describing the problem and a proposal to fix it. We then discuss the design and once the maintainers all agree with the design (more or less) you move onto creating a PR.
Sometimes maintainers won't agree, and it'll take writing a PR to convince them that it will work (this does happen). But in most cases, the design is the important part (if it's a non-trivial change).
The only counter-example I can think of is the Linux kernel. But that's an extreme example and usually a dummy PR will be enough to convince them to discuss your idea.
"Submit an issue or post to the mailing list before starting a significant amount of feature work so that your work might be wasted" is a pretty common refrain in open source circles.
That said, like most tribal knowledge, it's the people who already do a lot of this kind of thing that know this kind of thing, and it's getting new people in that's the tough part.
From the opposite point of view, before writing a significant patch/feature, it may be worth opening an issue with a quick description of what you'd like to implement to see if there's interest in it from the maintainers before investing the time in writing it. It may even be something someone is already working on in a private branch.
Even if it's something you really need and plan on maintaining your own fork if rejected, you could still get some tips on how to best implement it or potential tricky bits to be aware of.
I don't think compromising their own goals and ideals is going to help in the long run. I think it is better to underdeliver and focus on quality rather than merging most suggestions that come in.
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.
Down with the lowest tier of projects, I generally will always gratefully accept any help, since it doesn't come around very often. If the project is already pretty solid on tests and docs, I usually ask for them in the patch. Otherwise, I will do the grunt work there myself if need be.
You would think docker would take all the help they can get, given how bad their current code base is.
Maybe they should be "closing" their blog posts, and let real developers step in.
You're talking about a very popular project used in production by many people at many companies. If you're going to make a sweeping dismissal that it's terrible and all of the developers are bad, you absolutely have to back up your claims.
I 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."
It'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.
"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.
25 comments
[ 2.9 ms ] story [ 55.9 ms ] threadIf harder, it had better be a stunning new addition that makes the extra maintenance worth it... And often, it had better include an automated test!
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.
4 months ago: However, please read the notes in Roadmap and new features before continuing with this work, as I'm not yet sure whether or not they are within the scope of Umbrella
[...]
4 months ago: I don't think it's a good idea to change it, so only Strings will be available for after, before, etc.
3 months ago: I opened it to implement what was suggested here.
3 months ago: Okay, it was added.
[1] https://github.com/umbrellajs/umbrella/issues/20
Granting commit access is different level.
How we structure work on these projects needs to be rethought so that the majority of these don't happen. The attitude of code-or-gtfo is kinda broken with respect to how much work people put into a project with an uncertain outcome.
Maybe we should
Just communicating through pull requests seems very macho and wasteful to me.And maintainers will put many, many more hours into maintaining it. The submitter only has to interact with us once. We have to interact with their code for a long time.
> How we structure work on these projects needs to be rethought [...]
Most projects work the way you described (including Docker). You open an issue (and/or send a mail to the mailing list) describing the problem and a proposal to fix it. We then discuss the design and once the maintainers all agree with the design (more or less) you move onto creating a PR.
Sometimes maintainers won't agree, and it'll take writing a PR to convince them that it will work (this does happen). But in most cases, the design is the important part (if it's a non-trivial change).
The only counter-example I can think of is the Linux kernel. But that's an extreme example and usually a dummy PR will be enough to convince them to discuss your idea.
That said, like most tribal knowledge, it's the people who already do a lot of this kind of thing that know this kind of thing, and it's getting new people in that's the tough part.
Even if it's something you really need and plan on maintaining your own fork if rejected, you could still get some tips on how to best implement it or potential tricky bits to be aware of.
I would definitely not follow their model of open source development as they are an example of the opposite and equally ineffective extreme.
Underdelivered features at that scale is to be expected — quality is a bar that can't be compromised.
> 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."
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.
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.