- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 33.6k
meta: merge TSC and CTC back into a single body #14973
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
Conversation
| LGTM | 
    
      
        1 similar comment
      
    
  
    | LGTM | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are conflicts between the procedure here and as defined in the TSC repo (and charter). Those probably need to be resolved.
I highlighted via inline comments the ones that I noticed, there could be more.
        
          
                GOVERNANCE.md
              
                Outdated
          
        
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This says 1/3, TSC charter says 1/4.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps just update to 1/4 everywhere, if this fits with the current members set?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would need to decide. I prefer the 1/4 limit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1/4 looks better, especially if it doesn't contradict the current memebers list.
Also, preferring 1/4 would not cause a TSC charter change (and requesting approval from the board), and preferring 1/3 would.
        
          
                GOVERNANCE.md
              
                Outdated
          
        
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto (1/3 and 1/4 conflict)
        
          
                GOVERNANCE.md
              
                Outdated
          
        
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this in line with the TSC charter? I am not sure if I understand https://github.com/nodejs/TSC/blob/master/TSC-Charter.md#section-8-voting, but it was mentioned yesterday that any abstention could be counted as a vote against the resolution there.
I would like us to keep the behavior defined in this (CTC) document, though, where explitic abstaintions are subtracted from the total number of people voting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the TSC Charter requires a simple majority (50%+1) for all votes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can it be changed to allow explicit abstention?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason for that is that CTC is both larger and in a certain sense more fragmented than TSC.
There are various areas of expertise, and e.g. I find it normal when someone abstains from a vote in some area where they don't have enough prior knowledge and delegate that to those who do.
Voting in general and explicit consensus particularly are already hard enough for CTC, and I expect that forcing 50%+1 for all votes and not allowing explicit abstention will make those even harder, perhaps even non-functional in a significant number of cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it can explicitly allow for abstention.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To clarify — by «explicit abstention» I mean the same one which CTC uses, where the number of explicitly abstained people gets subtracted from the total number of people in the CTC in the percentage formula.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ChALkeR ... I have updated nodejs/TSC#317 to include a modification to the charter clarifying the effect of absention on the vote. I've updated this PR to point to the TSC Charter for details, including the one-quarter rule.
| Overall, I am in favor of this, but currently there are conflicts that need to be resolved. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clarity in organizational hierarchy and coherence in roles & responsibilities - sounds great to me.
Issues mentioned in the review are fixed not
| Argh, I can't edit the dismiss message.  | 
| I am in favor of this. | 
| @Fishrock123 ... does that count as signoff? If so, can I ask you to please use the Approve/Request Changes workflow as a clearer indication. | 
        
          
                COLLABORATOR_GUIDE.md
              
                Outdated
          
        
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be tsc-review?
        
          
                README.md
              
                Outdated
          
        
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/TST/TSC/
| @evanlucas ... updated | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with a nit
        
          
                GOVERNANCE.md
              
                Outdated
          
        
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/TST/TSC/
        
          
                GOVERNANCE.md
              
                Outdated
          
        
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Micro-nit: The link should probably go to ./README.md#tsc-core-technical-committee which will link to the same place as https://github.com/nodejs/node#ctc-core-technical-committee one the ctc is changed to tsc in the README doc. Totally not a blocking objection, just a tiny suggestion.
        
          
                GOVERNANCE.md
              
                Outdated
          
        
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This paragraph is now redundant because it contains information already included in the TSC Charter. It's still good to have it here, I think, but it may be worth including a sentence somewhere in this doc that notes that if anything written in this doc is in contradiction with something in the TSC Charter, the TSC Charter takes precedence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Also, that's another Totally Non-Blocking Suggestion.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That can be handled in a separate PR.
        
          
                README.md
              
                Outdated
          
        
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change means we'll need to merge the two WORKING_GROUPS.md docs. If anyone's feeling ambitious and wants to open a PR against the TSC repo to have that ready to go, that would be awesome. (Non-blocking, just pointing out something that someone may choose to act on at this time or not.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the corresponding PR against the TSC repo already does that.
PR-URL: #14973 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Shigeki Ohtsu <[email protected]> Reviewed-By: Fedor Indutny <[email protected]>
| Landed in f3eb193 | 
| Is there going to be a CTC meeting tomorrow morning? | 
| Yep, the CTC meeting schedule doesn't change. | 
PR-URL: nodejs/node#14973 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Shigeki Ohtsu <[email protected]> Reviewed-By: Fedor Indutny <[email protected]>
PR-URL: nodejs/node#14973 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Shigeki Ohtsu <[email protected]> Reviewed-By: Fedor Indutny <[email protected]>
PR-URL: nodejs#14973 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Shigeki Ohtsu <[email protected]> Reviewed-By: Fedor Indutny <[email protected]>
PR-URL: #14973 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Shigeki Ohtsu <[email protected]> Reviewed-By: Fedor Indutny <[email protected]>
PR-URL: #14973 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Shigeki Ohtsu <[email protected]> Reviewed-By: Fedor Indutny <[email protected]>
PR-URL: #14973 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Shigeki Ohtsu <[email protected]> Reviewed-By: Fedor Indutny <[email protected]>
Merge the CTC and TSC back into a single body. Dependent on approval and landing of nodejs/TSC#317
/cc @nodejs/ctc @nodejs/tsc