Comment by fourthark
7 years ago
Not a serious problem with "fire and forget", but an annoyance: say the patch is not up to standards: not well designed, needs tests, or whatever. It worked for the contributor but it can't be merged in its current form.
Now the maintainer has to explain what's wrong to a contributor who isn't going to do anything about it. And then explain again and again to everyone else who wants that feature that no, it needs work and can't be merged yet.
(Yes, I am that maintainer with dozens of open PRs on his repo, none so bad they must be closed, but also not good enough to merge.)
"(Yes, I am that maintainer with dozens of open PRs on his repo, none so bad they must be closed, but also not good enough to merge.)"
Go ahead and close them with notes that they can be re-opened if they come up to standards, but that you don't have the time or interest or capability to do it yourself, and that you are not judging them beyond this comment.
Closing a PR doesn't mean as much as people kinda think it does; they can be re-opened. Closing them without comment is harsh, but it correctly represents the current state of the world, which is that without changes, they aren't going in. Closing them with comment is the best compromise between all the issues.
As a maintainer, you need the "open PR" list to be a list of meaningful, actionable items. You don't have all that many tools to use, and the ones you do have need to be kept sharp. It's OK.
This. If you have 20 open PRs and all of them are old, they should be closed. Same for issues if they don't appear to have any resolution or are too stale.
If people keep asking for a feature that isn't ready, you can make an issue that depends on the PR that describes the acceptance criteria for the feature, and send them the link. Or you can edit the top of the PR description with the acceptance criteria/status so new people see it immediately.
A third option is to refuse issues and PRs entirely and just use mailing lists. Less people will ask for a feature because it takes a lot more work to dig through mailing lists, and by that time you've already found the thread and know why it isn't accepted yet.
On a technical level, I think this encourages the contributor to delete the branch, in which case it's gone for good and benefits no one.
If GitHub automatically saved a copy of each PR branch, I would be more receptive to this advice. This would potentially cause a lot of other problems...
Then again, why should you wait for the original submitter for those modifications? The original contributor published his code, and has not given any indication he's interested in the rest of the process. Why can't anyone else who wants that feature pick up the slack? Maybe you can make that explicit, marking such feature requests as NEEDS_WORK or something?
This is where the behavior of the maintainer is key.
On one hand, if the maintainer asks the contributor to fix up their code, the maintainer can be seen us ungrateful, demanding, dictatorial, and/or pedantic.
On the other hand, if the maintainer makes the changes herself, they can seem impatient, uncooperative, overpowering, and/or power-hungry.
Regardless where on the above spectrum the maintainers behavior falls, they can come across as friendly, neutral, pessimistic, or toxic. That depends on how they communicate. It also depends on the social norms that the maintainer and contributor are used to.
A maintainer must be extremely vigilant and aware of the tone they use and the image they want to portray. The contributor should also be aware of these things. If either side wavers, opportunities for an unhealthy brew arise.
This is compounded by the fact that most people, maintainers and contributors, are doing the work on a volunteer basis. Often there is an unspoken expectations that others should be grateful for the work that they are doing. When these expectations are not met, sour feelings arise.
Yes.
Given how difficult this is, it's no surprise to me that potential contributors might not want to stick their neck out, as expressed in TFA.
Yes, I find it helps to say "here is what is needed, anyone should feel free to take it from here." In my first response, so it's clear I don't expect anything more.
Quite a few PRs were improved and merged this way, and I think the feeling of community is enhanced when a PR goes through multiple revisions by multiple contributors.
> dozens of open PRs on his repo, none so bad they must be closed, but also not good enough to merge
But you've decided to reject them. Isn't that just what 'closed' is for?
They're not rejected. They're just not ready.
If it were something really stupid or wrong, I'd close it, sure.