zspencer
commented
6 years ago I agree with you. That said, we're very early in the project and I would really hesitate to add additional gates at this time. This is both because as you mentioned, early projects often bring in new stuff and because adding libraries sans code changes means we'd be making decisions about a libraries utility without the context of how they impact the code structures. Once we have more code, I think it'd make sense to request that when adding new libraries or doing refactorings we submit the change independently to see how it impacts the code. (I think we'll probably reach that point in the next week).
I've done a lot of work with competent beginner programmers, and the number one thing that prevents them from contributing to open source projects has been that they're not sure how to get past the waypoints to get their work merged. The more waypoints we add, the smaller adoption we'll get from outside contributors. This is why so many OSS contributors suffer from burnout despite having popular projects. They have high standards for what gets merged which is good from a technical perspective, but drives away people who are interested in moving the project forward.
For now, I'd prefer we err on the side of merging so contributors feel the time and attention they're giving freely is valued. We can always request changes on a PR when there is something we really don't like in a PR; so long as we're being specific, actionable and kind.
As an aside, I'm not sure I like characterizing contributions that include decisions we don't immediately agree with as thoughtlessness. It puts significant burden on contributors to "guess" what we would want or risk losing both the time and attention they invested in making a change and social standing because "OMG, Zee thinks I'm an idiot for using X instead of Y".
TL/DR: I think we will want to add firmer waypoints for contributing when we start to reach the point where we are unable to make correcting adjustments ourselves, so as to reduce labor burden and risk to contributors. Further, I'd prefer our core contributors focused on providing specific, actionable, and kind feedback on particular patches that center the contributor as a competent, valued, helpful person regardless of how much we agree with their implementation decisions.
I'm noticing a PR structure pattern where there's incidental infrastructure pulled in (
decent_exposure, rspec feature tests) alongside actual feature work, and the entire PR is then squash-merged onto master. I am uncomfortable with this pattern for the following reasons:decent_exposureand rspec feature tests, but if I raise those concerns on a feature PR then all of a sudden I'm blocking the feature. I don't like that social dynamic. It creates a situation in which people can barrel over other people with "finished code wins" and discourages thoughtfulness.I think that there may be an assumption that this is temporary because the project is new, and that it will settle down as the project matures. In my experience, that does not happen naturally, it happens because people choose to be intentional about PR size and scope norms. I would like us to set strong norms around patch size and scope early. My preferred norms -
I'm also open to alternate processes for reducing the size of commits on master, like rebase-focused processes, but those only address my commit size concerns. I'd like us to find a solution that also addresses my comms structure concerns.
TLDR: I think this project will be best served if we prefer thoughtfulness, historical record of decisionmaking, and transparency of values over velocity, and I see our current PR strategy as not doing that.