Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential flaw in the governance model of Node.js #1680

Open
anonrig opened this issue Jan 22, 2025 · 14 comments
Open

Potential flaw in the governance model of Node.js #1680

anonrig opened this issue Jan 22, 2025 · 14 comments

Comments

@anonrig
Copy link
Member

anonrig commented Jan 22, 2025

I believe that the current governance model, defined in TSC charter https://github.com/nodejs/TSC/blob/HEAD/TSC-Charter.md#section-7-voting, is some-what flawed and in favor of blocking people by burying them with bureaucracy, and time.

When a pull-request is blocked, current governance model is extremely time-consuming if the contributor decides to pursue to land their pull-request. In most cases, in order to avoid or spend a lot of time on this matter, people tend to stop pursuing it.

Good scenario

In a good scenario, the flow of landing a pull-request is as follows:

  1. Contributor A opens a PR
  2. Contributor B and C approves the PR
  3. After 48 hours, the PR lands

or

  1. Contributor A opens a PR
  2. Contributor B leaves a request changes review and blocks a PR
  3. Collaborator A and Collaborator B try to reach consensus without involving the TSC
  4. After 48 hours and removal of the block, the PR lands

Bad scenario

But in a bad scenario where a consensus is needed, here is the flow:

  1. Contributor A opens a PR
  2. Contributor B leaves a request changes review and blocks a PR
  3. Contributor A adds tsc-agenda label
  4. TSC meeting discusses the changes
  5. Time
    - In worst case, the next TSC meeting is in 7 days, best case it is in 3 days
  6. Meeting
    - In best case, it takes 1 meeting to resolve the issue, in worst case X meetings. The best case also involves making a decision of having a TSC vote, whereas an issue can be in X amount of meetings without any hint of ending the discussion.
  7. In the case of: TSC meeting ending with a voting required decision:
  8. It takes 1-2 weeks on average to get a result out of a voting session.
  9. A TSC member removes the block
  10. Most probably a rebase is needed and a new CI run in order to land.

In this bad scenario, unless Contributor B responds in a timely manner, their block will always be effective unless a discussion is done within TSC. This ensures that the TSC meetings are 100 percent efficient. But in reality, a holiday season can stall a pull-request from landing as large as 2 months.


Think of this:

On a random pull-request, you need to find 2 people to "like"/approve your pull-request for it to land. But if the pull-request is blocked, you need to convince half of the people in TSC (give or take 10 people) to land a pull-request that just got blocked. I think this can be used to drive a people out of the project, or force people to silently quit.


If you look into a "bad scenario" where a pull-request is blocked, it takes 1 minute to reject a pull-request with any sort of clear reason (the validity and the discussion about it is outside of the scope of this issue), whereas it takes almost a month of publicity, meetings and discussions to land a particular pull-request.

I think this is flawed and the process is in favor of people blocking pull-requests rather than contributing/landing a particular change.


I don't have a concrete recommendation other than stating the problem, but

  • We might increase the blocked reviewer limit to 2, rather than 1. Ex: It takes 2 "request-changes" to block a pull-request in 48 hours.
  • We can set a limit to the meeting count of a topic being discussed, and later force ourselves to take a vote. (This is still in favor of people blocking PRs but it will make the pain less and less...)
  • Please fill this...

cc @nodejs/tsc

@aduh95
Copy link
Contributor

aduh95 commented Jan 22, 2025

I feel a step is missing in the "Bad scenario" where, before anyone adds the label, Collaborator A and Collaborator B try to reach consensus without involving the TSC.

I like the idea of limiting the number of meetings an issue should be discussed, forcing us to be brief and hopefully more efficient would be a nice outcome.

@legendecas
Copy link
Member

I really don't like the tone in this issue that assumes the collaborator who request changes is not well intended and like @aduh95 mentioned that the consensus building step is omitted in the "bad scenario" before the escalation.

@RafaelGSS
Copy link
Member

Honestly, most of the issues/PRs that someone blocks are not simple to resolve, that's why it takes a reasonable amount of time. I don't see situations like that happening too often (except for node:test and smartOS discussion, of course).

Due to the nature of our project (maintained by volunteers) setting a time/deadline is very problematic as most of us don't get paid to work on that and use their free time to engage in discussions/prs. In the worst scenario, the PR won't land in X amount of time, and to be honest, that's better than rushing into a PR that can affect thousands of applications and then need an urgent hotfix from a nodejs releaser.

@anonrig
Copy link
Member Author

anonrig commented Jan 23, 2025

I feel a step is missing in the "Bad scenario" where, before anyone adds the label, Collaborator A and Collaborator B try to reach consensus without involving the TSC.

@aduh95 I've added it to the Good Scenario.

I really don't like the tone in this issue that assumes the collaborator who request changes is not well intended and like @aduh95 mentioned that the consensus building step is omitted in the "bad scenario" before the escalation.

@legendecas Sorry to hear that. I've tried to remove intention wise adjectives/sentences from this as best as I can since the intention is not the issue I want to raise. Can you give me solid recommendations to remove the places where you think "intention" is misused? Or can you update the issue directly since you also have the permission to do so.

@anonrig
Copy link
Member Author

anonrig commented Jan 23, 2025

Due to the nature of our project (maintained by volunteers) setting a time/deadline is very problematic as most of us don't get paid to work on that and use their free time to engage in discussions/prs. In the worst scenario, the PR won't land in X amount of time, and to be honest, that's better than rushing into a PR that can affect thousands of applications and then need an urgent hotfix from a nodejs releaser.

@RafaelGSS This very specific reason is the root cause of why people stop pursuing in the first sign of "review changes". Most TSC members are not paid to work on Node.js whereas most collaborators are also not paid to work on Node.js project. This is why we should at least think about the other end of the table. We should at least brainstorm about how to even the table for both good and bad scenarios outlined in this issue.

@legendecas
Copy link
Member

Dissent comments alone don't constitute an objection. Any pull request objection must include a clear reason for that objection, and the objector must remain responsive for further discussion towards consensus about the direction of the pull request

The collaborator guide to request a change stated that the objection shall be clarified and a pure objection is not valid.

And, in the explainer above, from the "bad scenario",

If you look into a "bad scenario" where a pull-request is blocked, it takes 1 minute to reject a pull-request with any sort of opinion/reason (a valid reason is somewhat vague to define in this context)

I found this assuming that the collaborator who requests change is hostile.

@anonrig
Copy link
Member Author

anonrig commented Jan 23, 2025

I found this assuming that the collaborator who requests change is hostile.

@legendecas Ah I see. Sorry for the confusion I've created. I was trying to say: "it's hard for me to define a "valid" reason at the moment, and it's outside of the scope of this issue".

Edit: I've updated the issue. LMK if it's better or worse.

@joyeecheung
Copy link
Member

joyeecheung commented Jan 23, 2025

IIUC, the meeting part is not a must. There were multiple TSC members (or CTC members back in the CTC days) that I had never seen in meetings before. It's technically possible to just start a vote.

I think consensus building in a democratic fashion just takes time and it's inevitable. This is not limited to software development, but also applies to governments around the world. It's not exactly news that democratic governments can get completely dysfunctional due to unresolvable disagreement until one party gives in, and these are people who get paid very well to do this. The only way to remove that risk of inefficiency is to just give up on democracy, it's a trade-off.

@mcollina
Copy link
Member

@anonrig let's assume both collaborators have the well-being of the project in mind, they tried to resolve the matter, and the objection is well-framed. Even if they share this common desire, they think the project should evolve in different directions, and those two directions leave no possible compromise. The TSC meeting is not a MUST either, but I don't see how to resolve conflict without one, as I assume the TSC would be essentially split into two camps.

The project needs volunteers to run, and therefore, picking one or another is very serious, because by choosing one side, the project always loses as the chance that a losing collaborator stops collaborating is not nil; they might even consider forking. The governance is set up to force people to collaborate and find compromise.

There are a few edge cases:

  1. B is hostile towards A.
  2. the TSC would ~unanimosly side with A or B.

We haven't seen many of these; however, we are always too slow to act when they appear.

In other terms, I propose we start doing a quick "Does anyone object to dismissing the block?" whenever a block appears.

@ShogunPanda
Copy link
Contributor

I also don't like the idea of the "hostile contributor" idea. But, to be fair, I think @anonrig has, voluntarily or not, brought up a point in assuming they exist or might exist. We all know which world we live in, unfortunately.

I also agree that in the TSC we are not the fastest to take action but usually it is for a good reason. But that does not imply that in some situation we are unnecessarily slow (especially since we mostly do in our free time).

In short: while our model is pretty solid, I also agree we should have a couple of fast paths to unlock objected PRs (possibly in a hostile way).

I propose two ways, which can be implemented alone, sequentially or together:

  1. "Does anyone object to dismissing the block?" (the one Matteo proposed) - No explicit responses in 5 days will constitute a "green light".
  2. "Request for TSC intervention to unblock" - It will be different from the current model as it will not escalate to a TSC meeting but it will be handled in the PR directly. 3 TSC upvote to the request will constitute a "green light".

In both cases, the PR will resume the 48 hours grace period and then it will be free to go. If no consensus is reached this way, the PR can still be brought up to the next TSC meeting as it is today.

@aduh95
Copy link
Contributor

aduh95 commented Jan 23, 2025

In other terms, I propose we start doing a quick "Does anyone object to dismissing the block?" whenever a block appears.

I don't know how effective that would be, I'm not sure I can think of a single instance where I wouldn't object to be dismissive of a block – blocking out of spite is very rare I think, and as you said above, the TSC cannot really afford to "take a side" without giving all the involved parties opportunity to make their case – failing to do so would very likely result in "the other side" leaving the project.


If you look into a "bad scenario" where a pull-request is blocked, it takes 1 minute to reject a pull-request with any sort of clear reason (the validity and the discussion about it is outside of the scope of this issue)

Worth considering the social cost of blocking a PR, e.g. blocking one of your PR, Yagiz, is not at all a thing I would take lightly because you have a much bigger social media audience than me, you're arguably better than me at presenting your case to the TSC, and I don't want to alienate you, especially considering you've been very vocal about how having your PR block makes you feel. I would love to believe that we would be exempt from that kind of bias, and only review a PR on a technical PoV, not depending on who's sending it – but let's be real, we're not.

Also, the blocking party also need to invest time to defend their block, probably as much as the party fighting the block, if they're actually feeling strongly about it, so I don't really buy the idea that blocking a PR is much easier than not.


The only way to remove that risk of inefficiency is to just give up on democracy, it's a trade-off.

I very much agree with Joyee, there's a trade-off. It may be useful to consider as two competing goals for retaining collaborators and contributors:

  • folks need to feel heard (veto power are great for that IMO)
  • folks need to get stuff done (veto power kinda conflicts with that one)

We have to consider that current collaborators are OK-ish to put up with those rules; if we amend them to change the power dynamics, we may or may not lose some of them (who would no longer be OK with the rules), and we may or may not get new collaborators as a result, so it sounds to me a risky gamble.
I'm not saying that there are no structural changes that could be made that would improve the situation, I'm saying that expediting such matter doesn't sound like a satisfying solution to me.

@joyeecheung
Copy link
Member

joyeecheung commented Jan 23, 2025

"Does anyone object to dismissing the block?" (the one Matteo proposed) - No explicit responses in 5 days will constitute a "green light".

I think this is basically the process that we already have, just shortening the length to 5 days?

If the objection is not clear to others, another collaborator can ask an objecting collaborator to explain their objection or to provide actionable steps to resolve the objection. If the objector is unresponsive for seven days after a collaborator asks for clarification, a collaborator may dismiss the objection.

3 TSC upvote to the request will constitute a "green light".

I feel that 3 is too small a number, considering TSC quorum is 10-ish. Personally in the past I even wondered why the choice is only given to ~20 out of ~100 when we start a vote. If we reduce it to 3 out of ~100 I think this would further validate the complaints that there is an out-group even among the collaborators or that the project governance is too limited to the privileged few (I have heard complaints from both non TSC collaborators and non collaborators) - one might be convinced that forcing 20 out of 100 to vote is good enough a sample, but accepting that 3 out of that 20 putting a thumbs up is enough would be tougher to sell it as a democracy.

Personally I have only started a vote because my PR got blocked. But I would not be comfortable to dismiss the block with just 3 thumbs ups, that would make me feel like being unfair to that non-TSC collaborator. I would be a lot more comfortable if I get a majority support from a collaborator poll instead of TSC vote, but I can accept that a TSC vote is a good enough sample to represent the collective of the collaborators given the size of TSC is not too small and there are too many dormant collaborators.

@jasnell
Copy link
Member

jasnell commented Jan 24, 2025

While our process is not perfect, and often times frustrating, I actually don't believe there's much to change from a policy perspective.

Fundamentally the ideal model here is to focus on consensus building. One person blocking a PR is not a bad thing at all if the PR should be blocked for whatever reason and it is perfectly ok and acceptable for folks to disagree on whether any particular reason to block a PR is valid. Where the process breaks down is when people are unwilling to communicate, cooperation, compromise, and collaborate on a path forward. Patience and collaboration are the critical success factors here and I think we've gotten away from that. It's ok if some things just don't move as quickly as others.

@gireeshpunathil
Copy link
Member

not specific to this case, but my general opinion on the formal PR rejections:

the red cross has a psychological impact on the contributor. the reject action expresses the rigidity and a lack of collaborative intent from the reviewer, especially when used as the first response. this can be discouraging for new contributors.

it generates an emotional and procedural weight onto the contributor. they must now

  • defend their work
  • accommodate feedback, and
  • work towards alignment.

it might be business as usual for veterans some of who love criticism, but it alienates those who are still building confidence in the repo.

blockers could attempt a more dialog-driven approach before formally rejecting a PR:

  • clearly pin-point issue(s)
  • reason why it is a real concern
  • provide actionable suggestions
  • ask clarifying questions
  • proposing compromises

these can create a more inclusive atmosphere. the red cross may be used only as a last resort.

this aproach aligns with our values and goals: code review aims not merely for technical perfection but also for building a culture of shared ownership and growth. by emphasizing conversation over contention, lets ensure that every contributor feels valued and empowered!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants