I'm searching for resources about code review processes. Feel free to reply with your experiences too. I'm not interested in technical matters right now like how to automate parts of code review or your Opinion about the right size of a pull request. I'm asking about the social process of deciding when to merge. How do you ensure quality and keep development moving ahead without getting dragged down by disagreements? Boosts appreciated.
An interesting idea someone told me about during the Linux App Summit arts BOF was +2/-1. IIUC that means as soon as two people review a change, it gets merged, unless someone objects. That seems like a good way to keep development moving. But that leaves open the question of how to handle impasses where neither side is willing to compromise.
I'm thinking impasses should be resolved by a binding vote once the arguments for all sides have been presented and the discussion starts to go in circles. What decides the vote? Plurality? Should it be a simple one-person-one-vote system? Weighted voting for multiple options? Maybe it depends on the situation?
@be How important is it to get stuff done quickly? If you can leave a dispute long enough for the scope to change / a better solution to be discovered, you might not need to compromise.
@wizzwizz4 Getting things done very quickly isn't too important. But not having releases for over 2 years is unacceptable.
I've seen situations where a review is stuck or delayed. There is a very wide range of reasons why this happens (most frequent first):
a) reviewers take very long to review
b) contributors do not address comments
c) reviewers do not read the contribution
Real disagreement is rare. And I'm yet to see an instance where it could be resolved with a vote of some kind.
@dachary @humanetech See how 6 people immediately agreed on how the feature should work yet one person delayed everyone else for WEEKS until being overruled by a vote. https://github.com/mixxxdj/mixxx/pull/3698#issuecomment-797858487
@be The method I'm used to is "NOTUC" for "No Objections To Unanymous Consent?".
Someone might want it and noone must strongly oppose for a decision to pass. Yes this might slow things down, and allows stubborn people to have their way.
But as long as everyone believes in the project and wants it to progress I find it works quite well!
@alcinnz I don't think a fork is warranted. But there is one specific developer who repeatedly won't budge even when literally nobody else agrees so we need some formal process to move past these situations.
@alcinnz That's generally how Mixxx has operated and my experience is that trying to achieve unanimous consent devolves into weeks/months of circular arguments.
@be I think outside review should be encouraged indeed. It may also be a way to _get started_ with contributing!
People who aren't comfortable with the codebase can learn it faster that way.
Does any forge in existence allow outsiders to do a _formal_ review, rather than just leave a comment?
@dmbaturin Almost every time I ask new people to do code reviews they've said they don't know what they're doing.
@be Now that you say that, I think projects can benefit from code review checklists!
E.g. "I have tested it and it works for me with this data:", "This is a scenario that breaks this code", "I believe this may cause this problem in the future" etc.
@dmbaturin Most often what we really need doesn't even require coding skills: "This feature makes sense and I agree it will be a good improvement."
@be I guess you are right. I've only seen and done code review in high-responsibility projects that require professional reviewers to prevent bad things from happening, but many more projects could benefit from code review—it's just not critical enough for them to bear the costs requiring it.
@dmbaturin We need more than "I tested this and it works as described." We need "I tested this and I like the change it implements and it works as described."
@be Let's actually make a reusable checklist! I made a quick draft under CC0, available for contributions on Codeberg: https://codeberg.org/dmbaturin/code-review-checklist and Microsoft GitHub: https://github.com/dmbaturin/code-review-checklist
@be As much as I want to support alternatives, I'm realistic about the numbers of people who will register there just to contribute to it... it will take a long time before not being on GitHub will mean not sacrificing anything I'm afraid.
@dmbaturin People who won't contribute to something because it isn't on GitHub probably aren't worth collaborating with.
@be I wouldn't take such a stance. Contribution effort threshold is a real thing. When you already have an account and are familiar with the process, it's much lower.
A large, popular, and important project can tell people to sign up for a new forge account, a listicle definitely cannot.
and there was another project last month where i added a small thing but didn't want to use my GH account so fortunately they included an explicit option to email them
Fosstodon is an English speaking Mastodon instance that is open to anyone who is interested in technology; particularly free & open source software.