Request for the committer delegation team

[emilazy]

@Mic92 @NickCao @jtojnar

In light of the events in @phaer’s PR #392354, I request that the Nixpkgs committer delegation team consider revoking @SuperSandro2000’s commit privileges. It’s my understanding of the committer delegation team process that removal of committers falls under your jurisdiction, other than perhaps for Code of Conduct violations handled by the moderation team, which I do not feel is the appropriate venue for this matter.

I asked Sandro whether he intends to respond on that PR a few hours ago when he was active on Matrix but haven’t received any reply. I feel that there have been too many incidents lately where Sandro acts irresponsibly but it is shrugged off and forgotten about, so I am going ahead and posting this request rather than continuing to wait. There is seemingly no private point of contact for the team and I am not sure it is best to address these kinds of things behind closed doors, so I am posting this publicly here in a separate issue to avoid derailing either the PR or the committer interest list issue.

I understand that that the team may not feel comfortable handling the revocation of commit privileges or that it may be difficult to reach an unanimous decision, in which case I request that you state as much here so that this can be escalated to the Steering Committee if necessary.

I have provided my own reasoning and supporting evidence below, but I hope that Sandro will respond, and I invite anyone else to add anything they feel is important to the discussion. I hope I don’t need to say that discussion of this matter should remain civil, and I request that people strongly consider how their comments will come across before posting.

I don’t have extensive links to recent interactions, but I’ll list a few recent PRs where I think Sandro’s involvement has been problematic:

• ffmpeg_4: almost drop #336401

  This was a similar situation to the latest one: a PR I opened last August and assigned to (by accident, rather than requesting a review from) my two FFmpeg co‐maintainers. It contained a policy change around FFmpeg that I hoped for their feedback on. Sandro manually removed their assignments and merged it without comment.

  My co‐maintainer @Atemu’s attempt to call out this behaviour was met with frankly lame and insulting excuses implying that I (a contributor for half a decade at that point) had accidentally assigned them because I didn’t know what I was doing, and that a PR titled “ffmpeg_4: almost drop” wasn’t really about FFmpeg.

  To Sandro’s credit, he apologized after that. He didn’t respond to @Atemu’s further reply about the inappropriateness of the behaviour, though, so that thread of discussion ended with @Atemu’s final comment on the matter: “As a maintainer, I feel disregarded here and I regret showing support of restoring your merge privilege a little”.

• treewide: drop copumpkin from maintainers #392354

  Unfortunately, it seems to me that nothing was learned from the FFmpeg incident: here we have a PR with an explicit reply from another committer intending to delay merge for a week or two, in line with the written guidelines and previous controversy around PRs to remove inactive committers, but Sandro dived in to push a change that the PR author had already expressed disagreement with, with a commit message that violates our guidelines, and then merged the PR less than an hour later.

• Only setup nix-related tmpfiles if nix is enabled #343784

  Example of a fairly recent Sandro review, coming after a subject matter expert had already approved, consisting solely of stylistic nitpicks, and which nevertheless included a bad suggestion (a cosmetic reordering that would have resulted in incorrect indentation and inaccurate comments) and an incorrect complaint (pointing to the commit message guidelines despite them already having been followed). I don’t have the time to trawl through pages of reviews, so this is the only concrete example of a bad Sandro review I’ll give, but I think anyone who does do a more systematic review will find that this is not an outlier.

As always, I am less concerned about any individual mistakes here – it’s not good for the project if people become afraid to do anything for fear of having issues counted against them! – than about the lack of communication around them and the pattern they illustrate. It is often difficult to get him to admit fault, or even to comment on his actions at all, and I have not observed him significantly change his behaviour in response to his mistakes.

The sloppiness with regard to his personal contributions and actions as a committer is in stark contrast with the typical experience of having your own pull request reviewed by Sandro – there’s no way that a commit message like “Undo todo removal” would pass without comment from him on someone else’s PR, but in this case it was self‐merged within less than an hour – and it is there that I think the most harm is done.

I want to address the elephant in the room: Sandro is an extremely active contributor, the non‐robot with the second‐highest number of commits in the project, and still top 10 over the past year. We have a large PR backlog, and he reviews and merges a very high number of contributions. That’s a good thing for Nixpkgs, since the average contribution is a simple improvement that might just need some review attention to get over the finish line. Although I would consider myself fairly active, I definitely don’t have the stamina or patience to go through such a large volume of contributions. We need that kind of activity and I’m grateful for it.

All the same, however, merging a lot of PRs should not be a free pass to get away with behaviour that’s harmful to the project. There is review quantity, but there is also review quality, and Sandro is well‐known as an incredibly nitpicky reviewer. I think the average Sandro review tends to contain far more stylistic nitpicks than substantive issues, and that indeed he often misses the latter or brings up issues on subsequent rounds of review that he could have pointed out the first time around.

The result is a less welcoming Nixpkgs. I first saw complaints about his reviewing style in 2021, shortly after he became a committer. I know for a fact that Sandro has played a significant part in discouraging multiple people from contributing to the project. I was almost one of them; years ago, getting a review from Sandro on one of my pull requests was one of the things I dreaded most in Nixpkgs, and it factored into some of my earlier breaks from the project. Helping merge a lot of PRs is a good thing, but what about the PRs we’re not getting precisely because the review style of one of our most active committers encourages this culture of unsubstantive nitpicking and locking contributors into repeated rounds of satisfying a reviewer’s personal preferences?

These days, my commit bit gives me social capital and adds weight to my contributions and responses, and I have learned more about the politics of Nixpkgs contribution: I know that if I get an unhelpful Sandro review I can address the minor nitpicks, give my response to the things I disagree with or that are his personal opinion dressed up as established consensus, and carry on as usual.

But knowing who you can safely disregard reviews from should not be a necessary part of a healthy project’s contribution process. Non‐committers, especially new contributors, don’t have that luxury. They don’t know who is who, what kinds of reviews are truly blocking, and they don’t have the name recognition or GitHub permissions to make decisions about what reviews to respond to. Any review from any committer is essentially the final word to them unless another committer happens to notice the PR and take pity on them. These kinds of reviews disproportionately harm the people we should be doing most to encourage and nurture. This is especially problematic in combination with his tendency to ignore other committers, as exhibited in this most recent incident.

A common refrain I’ve heard from people in private is that they find him exhausting to interact with and think his commit access is long overdue to be revoked again, but feel that the project’s processes are too dysfunctional for it to be worth stating publicly. I think it is bad for people’s faith in the project if we cannot handle these situations; as we hand out commit access quite liberally, we must also be able to handle situations where it should be revoked, rather than letting our openness to inviting people become a one‐way ratchet. Not being able to have these tough discussions will only lead to the atmosphere becoming more toxic, with backroom venting and people feeling the need to work around processes rather than with them.

Sandro originally became a committer in November 2020. The commit bit was revoked by @zimbatm in July 2023 with the following reasoning:

I have removed SuperSandro2000’s commit privileges until we can figure out a better way to collaborate. I’m writing this here to be transparent about the decision-making process.

Nixpkgs is an experiment where we gave 197 persons commit access. We do have some tools and guidelines, but mostly, we count on each other’s suggestions to establish a standard way of doing things. This only works if the feedback goes both ways.

I want to avoid putting Sandro on blast as his actions are coming from a good place; to get things done. And that he did a lot. Unfortunately, making unilateral decisions is also pushing other contributors away. This is a signal that I keep getting since the start, so it’s time to act on it. Removing the commit access is also not a permanent decision, it’s a way to force a resolution to that issue.

He requested to become a committer again in December 2023, saying that the break helped him grow.

At the time, @JulienMalka asked:

Given that your commit bit was stripped because the way you acted was sometimes destructive to other contributors, could you elaborate on the commitments you intend to keep were we to give you a new chance with the commit bit? Having a "little break" is not enough, in my opinion you also have to explain why we should have reasons to trust you again, and that includes owning your past mistakes.

and @mweinelt commented:

My concern is that your throughput is very high, but as I recently pointed out, you are a lazy communicator, which often leads to misunderstandings and sometimes conflicts. One of the reasons your commit bit was previously removed was in part due to stepping over other people's reviews, partly due to a lack of proper attention.

Crucially, your application sounds like a fluff piece about your motivation and accomplishments, but fails to address any of the reasons, why your commit bit was revoked in the first place. I'm fully aware that this is hard in a public venue, but we kinda need to reestablish the will to collaborate with you on nixpkgs.

I’ll excerpt from his reply (sorry, feel free to click through and read the whole thing – I’m not trying to editorialize too much here, it’s just that this post is extremely long already):

I want to keep my throughput on a reasonable level, especially since other duties sometimes will require more attention and are higher priority. As you probably have noticed lately, I sometimes take a week or two to respond to PR feedback on my own PRs and implement it.

Also I want to prioritize high quality reviews with explanations over quantity. I think I got already tied less to attention to details (which was a problem for people before).

[…]

In the last ~6 months, I have still contributed to the project, patiently waited for reviews and merges and accepted my demotion but still trying to be part of the community and bring the fixes I did to all people.

I never disagreed with my mistakes and have silently accepted them and tried to work on them the best I can. I am still human and change needs time, hence I think the few months break where good to give the change a bit of time. I don't try to talk my mistakes down or bend the rules to make them look less sever.

[…]

I usual try to be to the point and compact in my writing, to not write overly long sentences no on can understand, even me in a few days. I learned that this is sometimes not good enough but language was never a strong skill of me. I am taking this as feedback to work on. Usually it is enough to give texts a second read and I should be doing that more often :)

[…]

As I already wrote above, I want to focus on more on high quality reviews and in question reach out to the community for help if I am unsure or there seems to evolve a conflict I don't know how to properly resolve.

In response to this, @JulienMalka commented again:

@SuperSandro2000 thank you for your answer. I believe in second chances and so I personally am favorable in giving you the commit bit back as long as you are keeping these commitments.
One last point that you have not cited and is still important is not to merge PRs when some people are still reviewing them / there are still some unaddressed points from other reviewers.

Sadly, that last point, also raised in @mweinelt’s comment, is exactly what we have run into again here.

In January 2024, @domenkozar responded to the request by making Sandro a committer again, saying “I strongly believe in setting the boundaries, enforcing them and giving people a second chance”. I agree with that; I’m in favour of second chances. I was on a break from Nixpkgs when Sandro was re‐added, but I’ve heard people say that at the time he had improved from how things were when his commit privileges were revoked.

However, in that case, it seems like there has been a regression now that the access was granted again. I heard many complaints about Sandro in 2024 along the same lines as always, and witnessed the same kinds of problematic behaviour firsthand. As I said, I’m in favour of second chances, and even third chances. But they have to be predicated on actual change, and if there is significant distance between Sandro in 2023 and Sandro today, I’m not seeing it, and I don’t think it’s enough. Any interaction with him still feels like you’re contributing to Sandropkgs.

Anyway, I didn’t want to write this, I didn’t enjoy doing so, and although I realize and regret that posting this will surely hurt Sandro (it would hurt me, too), I don’t want to make it personal. Communication is hard and any FOSS project has its conflicts. Clearly, Sandro has a lot of enthusiasm for the project and has dedicated an immense amount of time to trying to make it better, and I don’t believe he is acting maliciously. I have had frustrating interactions with him, but due to my aforementioned status I can largely shrug it off at this point and it doesn’t significantly impede my own contributions. But ultimately, I do think that he is not sufficiently attentive to his ability to cause harm to Nixpkgs, and being able to understand the difficulty of participating in a collaborative project doesn’t mean that we can just accept it when problems like these persist. It’s been over a year since his commit privileges were restored. What does it take before we admit that the second chance has failed?

[eclairevoyant]

I don't participate in nixpkgs much these days, but I could corroborate some similar interactions:

• Formatting nit from Jul 2024, back when I was active and a committer in nixpkgs: arma3-unix-launcher: init at 413 #324452 (comment). Not my PR, but by that point we actually had nixfmt-rfc-style as the CI-enforced formatter, no reason to be doing formatting nits like this instead of saying "hey just run nixfmt to satisfy CI". I at least could (and did) mark that thread resolved to avoid dissuading a first-time contributor.
• My own PR from a couple weeks ago, where the suggested changes were literally just "remove newline here, add it over there, because extra newlines are apparently bad... reasons.": nixos-install-tools: enable flakes when using nixos-generate-config --flake #387594 (review). I don't care about it getting merged, I maintain an immortal fork of nixpkgs for my own configs, but to do a drive-by nit and then not actually respond to the PR after I humoured the nit makes me feel like I wasted time making a PR.

[Mic92]

Forwarded this message to the internal matrix chat.

[piegamesde]

While making a public issue was the right move here IMHO, it usually won't be, therefore I'd appreciate if the commit bit team could establish some private points of contact in the future. For comparison, the moderation team is reachable on several platforms, which lowers the bar to reach out.