8 ms·

Some people clearly pay a great deal more attention to commit history than I ever do. The only difference this makes to me is that adding more hoops I have to j

Some people clearly pay a great deal more attention to commit history than I ever do. The only difference this makes to me is that adding more hoops I have to jump through before you'll accept my patch makes it less likely that I will bother to submit one in the first place.

richmarr 10y ago
I've let at least one PR go stale after having made the maintainer's requested changes, only to find a secondary list of demands such as squashing and 'comments too long'. Way to suck all the enthusiasm out of contributing.
- ams6110 10y ago
  I've had exactly the same experience. I submit a fix for a bug, OK clean up a few cosmetic things, resubmit, then get another list of nitpicks and I just dropped it there. I remember thinking "fuck it, clean it up yourself if you like, or just keep the bug."
  - ealloc 10y ago
    As a maintainer who sometimes requests PR submitters to make style changes or even squash their PRs: Please tell me you don't want to finish it so that I can do so. I am reluctant to merge your PR if it has too many style issues or improper commit history. The last 10% can be important when maintaining a large project. Because of the way git/github work, modifying someone else's PR (by pulling locally + modifying + resubmitting as a new PR) often means their authorship in the commit headers is erased, and I wouldn't want to do that without permission.
    - richmarr 10y ago
      Totally understandable, but I don't think project owners/maintainers behave quite as rationally as you're presenting here. 'Important', 'style issues', 'improper' and 'large project' are open to interpretation and ego comes into play. Edit: Not saying PR authors behave perfectly rationally either. Sometimes it's kids in long trousers failing to share their toys, and in the end "my ball my rules" wins.
geofft 10y ago
I do feel like this sort of thing should be the maintainer's responsibility. I'm pretty familiar with `rebase -i`, but only because I've done a bunch of work with git. Random contributors are as likely as not to be brand new to git. It should be reasonable to tell the maintainer, "Here's my branch, feel free to rebase or squash as you see fit." Of course, I think the same is true with things like comments, obvious coding style fixes, tests (especially if the maintainer has more access to test infrastructure), etc. I can see the advantage of getting people to do it themselves if the maintainer is busy or especially if the contributor is planning on sticking around for a while, but surely the maintainer can ask "Hey, would you <...>? If not, I can do it."
- panic 10y ago
  IMO, comments and tests should be written alongside the original code, not left up to someone else. If you don't have access to the test infrastructure, how did you test your code before submitting it?
  - geofft 10y ago
    In general, I test things in my environment, for the use case I have, which is the only way I can confirm that it fixes the actual problem I'm having. I'm not an expert in every test harness out there, and I generally expect the maintainer knows better than I do. But beyond that, for most projects, tests apply to multiple environments, and I'm only guaranteed to have mine. If a project is supported on Windows, Mac, and Linux, on Python 2.7, 3.3, 3.4, and 3.5, I probably have one, maybe two, of those twelve choices. It makes more sense to reuse the project's test infrastructure than for me to track down the other eleven configurations. And as a random contributor, I might not have access to the test environments. Of course, if the tests fail, that's a valid reason to push back on me (or for the maintainer to fix the problem and squash, if it requires access to an environment I don't have).
jonaf 10y ago
Regardless of your opinion, if you have any interest in code quality (hint: you should), then you should respect the contributing guidelines for the project you are contributing to. Your argument / opinion as stated could have "commit history" replaced with "unit tests" and in many circles (for some reason), people are still making that argument. For a project to be healthy, a critical eye must be used for every change: from the code, to the tests, to the commit message. If you don't care about any one of these, you are saying you don't care about the quality of the software. I would deny your PR on principle. My project needs more quality than it needs more contributors. Note that I did not suggest squashing is good or bad. My opinion on that is not related to the fact that you should care about it and treat it as just as important as your contribution, and you should respect the project enough to abide their guidelines for contribution (unless your contribution is revising those guidelines).
- marssaxman 10y ago
  Of course. I'm not talking about projects where I have already made a commitment to participate, though: I'm talking about the process before that, where there's some open-source project I am using or otherwise interested in, and I consider getting involved and contributing. The higher the initial barrier to entry, the more likely it is that I'll just shrug my shoulders and say "fuck it". A complex process for contributing changes, with lots of unusual fiddly steps about exactly how commits should be formatted, makes it more likely that I won't ever make that first commit - especially because the first commit tends to be something trivial where it's as much about dipping my toe in and seeing if I want to do something as about the actual change I've made. So, yes, I certainly do respect the project enough to abide by their guidelines for contribution: but if those guidelines demand too much work of me, that respect will take the form of quietly walking away and spending my time on something else, and the maintainers of the project will likely never know I had any interest in helping to begin with. Guessing that there might be other people like me in the world, then, I suggest that having a complex and specific process for open-source contribution is likely to reduce the pool of devs willing to get involved, and it's worth considering whether the amount of work saved by requiring devs to jump through these hoops outweighs the obstacle that creates for recruitment.

richmarr 10y ago

I've let at least one PR go stale after having made the maintainer's requested changes, only to find a secondary list of demands such as squashing and 'comments too long'. Way to suck all the enthusiasm out of contributing.

ams6110 10y ago
I've had exactly the same experience. I submit a fix for a bug, OK clean up a few cosmetic things, resubmit, then get another list of nitpicks and I just dropped it there. I remember thinking "fuck it, clean it up yourself if you like, or just keep the bug."
- ealloc 10y ago
  As a maintainer who sometimes requests PR submitters to make style changes or even squash their PRs: Please tell me you don't want to finish it so that I can do so. I am reluctant to merge your PR if it has too many style issues or improper commit history. The last 10% can be important when maintaining a large project. Because of the way git/github work, modifying someone else's PR (by pulling locally + modifying + resubmitting as a new PR) often means their authorship in the commit headers is erased, and I wouldn't want to do that without permission.
  - richmarr 10y ago
    Totally understandable, but I don't think project owners/maintainers behave quite as rationally as you're presenting here. 'Important', 'style issues', 'improper' and 'large project' are open to interpretation and ego comes into play. Edit: Not saying PR authors behave perfectly rationally either. Sometimes it's kids in long trousers failing to share their toys, and in the end "my ball my rules" wins.

geofft 10y ago

I do feel like this sort of thing should be the maintainer's responsibility. I'm pretty familiar with `rebase -i`, but only because I've done a bunch of work with git. Random contributors are as likely as not to be brand new to git. It should be reasonable to tell the maintainer, "Here's my branch, feel free to rebase or squash as you see fit." Of course, I think the same is true with things like comments, obvious coding style fixes, tests (especially if the maintainer has more access to test infrastructure), etc. I can see the advantage of getting people to do it themselves if the maintainer is busy or especially if the contributor is planning on sticking around for a while, but surely the maintainer can ask "Hey, would you <...>? If not, I can do it."

panic 10y ago
IMO, comments and tests should be written alongside the original code, not left up to someone else. If you don't have access to the test infrastructure, how did you test your code before submitting it?
- geofft 10y ago
  In general, I test things in my environment, for the use case I have, which is the only way I can confirm that it fixes the actual problem I'm having. I'm not an expert in every test harness out there, and I generally expect the maintainer knows better than I do. But beyond that, for most projects, tests apply to multiple environments, and I'm only guaranteed to have mine. If a project is supported on Windows, Mac, and Linux, on Python 2.7, 3.3, 3.4, and 3.5, I probably have one, maybe two, of those twelve choices. It makes more sense to reuse the project's test infrastructure than for me to track down the other eleven configurations. And as a random contributor, I might not have access to the test environments. Of course, if the tests fail, that's a valid reason to push back on me (or for the maintainer to fix the problem and squash, if it requires access to an environment I don't have).

jonaf 10y ago

Regardless of your opinion, if you have any interest in code quality (hint: you should), then you should respect the contributing guidelines for the project you are contributing to. Your argument / opinion as stated could have "commit history" replaced with "unit tests" and in many circles (for some reason), people are still making that argument. For a project to be healthy, a critical eye must be used for every change: from the code, to the tests, to the commit message. If you don't care about any one of these, you are saying you don't care about the quality of the software. I would deny your PR on principle. My project needs more quality than it needs more contributors. Note that I did not suggest squashing is good or bad. My opinion on that is not related to the fact that you should care about it and treat it as just as important as your contribution, and you should respect the project enough to abide their guidelines for contribution (unless your contribution is revising those guidelines).

marssaxman 10y ago
Of course. I'm not talking about projects where I have already made a commitment to participate, though: I'm talking about the process before that, where there's some open-source project I am using or otherwise interested in, and I consider getting involved and contributing. The higher the initial barrier to entry, the more likely it is that I'll just shrug my shoulders and say "fuck it". A complex process for contributing changes, with lots of unusual fiddly steps about exactly how commits should be formatted, makes it more likely that I won't ever make that first commit - especially because the first commit tends to be something trivial where it's as much about dipping my toe in and seeing if I want to do something as about the actual change I've made. So, yes, I certainly do respect the project enough to abide by their guidelines for contribution: but if those guidelines demand too much work of me, that respect will take the form of quietly walking away and spending my time on something else, and the maintainers of the project will likely never know I had any interest in helping to begin with. Guessing that there might be other people like me in the world, then, I suggest that having a complex and specific process for open-source contribution is likely to reduce the pool of devs willing to get involved, and it's worth considering whether the amount of work saved by requiring devs to jump through these hoops outweighs the obstacle that creates for recruitment.