Seen from the other side
Seen from the other side
Posted Feb 7, 2025 6:03 UTC (Fri) by wtarreau (subscriber, #51152)Parent article: The selfish contributor revisited
What I found in this case is to address yourself the small unimportant stuff (e.g. fix typos or indent), mentioning it when taking the change and presenting it as something normal, so that the person is not surprised to see their contribution modified, and if there's nothing to fix but you're not much happy with the change, but have no technical argument against it (often we're driven by feeling itself driven by experience), just openly mention it: "I sense something might go wrong by proceeding like this, but I can't tell what; since I have no technical argument, let's merge it, and if later we figure it really causes a problem, we can revert it". It generally makes the contributor more careful in the future about the goals of the project, and if the suspected problem finally happens and something needs to be changed, the person doesn't perceive it as something bad since it was already envisioned. Usually that makes that contributor feel trusted and come back with more contributions. And even the problems you envisioned are often related to a slight change of direction that was clashing with your approach, but the contributor can continue that change and finally replace an older approach with something much better in the end.
This is a good example of long-term win/win driven by trust. It's difficult though because as a project leader you're also the one responsible for the project not to steer in directions that don't match the community's expectations, but by making public statements like this, you bring the community with you, or the community will contest your decision and you can roll it back if there are good reasons.
Posted Feb 8, 2025 0:16 UTC (Sat)
by mathstuf (subscriber, #69389)
[Link] (7 responses)
Why not have this done in an automated way? It relieves the reviewer from having to consider it and the contributor can work with CI before looping a human into the loop.
Posted Feb 8, 2025 1:36 UTC (Sat)
by neilbrown (subscriber, #359)
[Link] (6 responses)
Because small-talk is important. It can help build connection and trust.
Posted Feb 8, 2025 12:03 UTC (Sat)
by wtarreau (subscriber, #51152)
[Link] (5 responses)
Everyone prefers being explained by a human why their work was sub-perfect and was fixed rather than having a bot reject it for obscure reasons.
Also when you see the same shocking patterns happen often in submissions, you can start to think about documenting them or even relax your coding style if that's not dramatic.
Posted Feb 9, 2025 1:37 UTC (Sun)
by mathstuf (subscriber, #69389)
[Link] (4 responses)
In my case, the robot can also *do* the formatting fixes (I also don't like the "CI says formatting is wrong" that doesn't offer a fix or at least a diff I can apply), so there's no real "rejection" here nevermind for "obscure" reasons.
My experience is that it allows me more time as a reviewer to focus on the actual technical content of the patch rather than nitpicking over minutae (which is attention-consuming and a great way to make very-chatty PR threads…GitLab at least allows collapsing things usefully though; I don't know how Github-based reviews deal with its behaviors there).
Even as a contributor, I like using CI to improve my patches before roping a human into the loop.
Posted Feb 9, 2025 15:16 UTC (Sun)
by Wol (subscriber, #4433)
[Link] (3 responses)
Those people who don't like the project's default format (and I would definitely enforce one) should simply define their own formatter to run on checkout. Then everybody wins - code in the repository is consistently formatted to the repository's rules, and contributors work with code formatted to their rules.
Cheers,
Posted Feb 9, 2025 16:15 UTC (Sun)
by mathstuf (subscriber, #69389)
[Link] (2 responses)
In any case, I have kind of inured myself to many objectionable formatting quirks in projects I drive-by contribute to (e.g., GNU-style indentation or inter-line alignment in tab-based projects) and other times I'm just not familiar enough with the language to know how some idiom should be formatted (e.g., comment placement/wrapping or Ruby's closures). No, the issue is that I have better things to focus on when coding than whitespace tweezing or line wrapping rules; these are things we *can* reliably outsource to automation. I hope that reviewers do too.
Posted Feb 9, 2025 22:19 UTC (Sun)
by wtarreau (subscriber, #51152)
[Link] (1 responses)
Posted Feb 10, 2025 7:38 UTC (Mon)
by mathstuf (subscriber, #69389)
[Link]
[1] https://git-scm.com/docs/git-config#Documentation/git-config.txt-blameignoreRevsFile
Seen from the other side
Seen from the other side
"I took your patch seriously and here are some comments" says something quite different to "nobody bothered to look at your patch because you misspelled 'colour'."
Seen from the other side
Exactly! It's super important IMHO to build trust with the community.
Seen from the other side
Seen from the other side
Wol
Seen from the other side
Seen from the other side
Seen from the other side
