Vote on primordials in error path

[aduh95]

I've found the subject of voting in TSC comes quite often, and we often have recurrent discussions on what tool to use. I've decided to take upon me the creation of a tool to help us vote using CLI and GitHub UI. It might end up being a terrible idea, or a genius one, but at least we would have try. If it's successful, we can talk about officially adopt this tool for Node.js related votes.

The vote options were presented in #1104 (comment).

Use one of the following methods to case a vote (or abstain):

Vote using npx+gh (requires Node.js 16+, and git)

Requires gh (GitHub CLI tool) to be installed, and logged in to your GitHub account.

npx --package=@aduh95/caritat voteOnGitHub https://github.com/nodejs/TSC/pull/1158

If you want to abstain, you can use the --abstain flag (it will create a ballot with all the scores set to 0):

npx --package=@aduh95/caritat voteOnGitHub https://github.com/nodejs/TSC/pull/1158 --abstain

Vote using npx (requires Node.js 16+, and git)

# Cloning and pushing using SSH
npx --package=@aduh95/caritat voteUsingGit -r git@github.com:nodejs/TSC.git \
  -b vote-primordials-error-path -p votes/primordials-error-path

# Cloning and pushing using HTTPS
USERNAME=<your-github-handle> \
npx --package=@aduh95/caritat voteUsingGit -r https://$USERNAME@github.com/nodejs/TSC.git \
  -b vote-primordials-error-path -p votes/primordials-error-path --handle "$USERNAME"

# To abstain, use the flag --abstain
npx --package=@aduh95/caritat voteUsingGit -r git@github.com:nodejs/TSC.git \
  -b vote-primordials-error-path -p votes/primordials-error-path --abstain

Vote using sh (requires git, and OpenSSL CLI)

Caution: LibreSSL CLI is not compatible, please use OpenSSL 1.1.1+.

curl -LOJ https://github.com/stduhpf/caritat/raw/HEAD/sh/voteUsingGit.sh
curl -LOJ https://github.com/stduhpf/caritat/raw/HEAD/sh/encryptBallot.sh
chmod +x encryptBallot.sh
sh ./voteUsingGit.sh \
  <your-github-handle> \
  git@github.com:nodejs/TSC.git \
  vote-primordials-error-path \
  votes/primordials-error-path
rm encryptBallot.sh
rm voteUsingGit.sh

Vote using PowerShell (requires git, and OpenSSL CLI)

Caution: LibreSSL CLI is not compatible, please use OpenSSL 1.1.1+.

Set-ExecutionPolicy Bypass -Scope Process -Force
[System.Net.ServicePointManager]::SecurityProtocol = [System.Net.ServicePointManager]::SecurityProtocol -bor 3072
$tmpFile = $(New-TemporaryFile | Rename-Item -NewName { $_ -replace 'tmp$', 'ps1'} -PassThru)
$encryptBallot = $tmpFile -replace '\\[^\\]+$','\encryptBallot.ps1'
((New-Object System.Net.WebClient).DownloadString('https://raw.githubusercontent.com/stduhpf/caritat/HEAD/sh/voteUsingGit.ps1')) | Out-File -FilePath $tmpFile
((New-Object System.Net.WebClient).DownloadString('https://raw.githubusercontent.com/stduhpf/caritat/HEAD/sh/encryptBallot.ps1')) | Out-File -FilePath $encryptBallot
& $tmpFile `
  <your-github-handle> `
  git@github.com:nodejs/TSC.git `
  vote-primordials-error-path `
  votes/primordials-error-path
Remove-Item $encryptBallot
Remove-Item $tmpFile

Vote using the web

1. Fill in the ballot at https://stduhpf.github.io/caritat/#https://github.com/nodejs/TSC/pull/1158. It will copy the encrypted ballot to your clipboard.
2. Paste the encrypted ballot in https://github.com/nodejs/TSC/new/vote-primordials-error-path/votes/primordials-error-path. It's recommended to use <your-github-handle>.json for the file name.
3. Commit directly to the vote-primordials-error-path branch.

To abstain, set all scores to 0 before encrypting the ballot.

This will encrypt a file containing your preferences, file that will be decrypted when doing the counting in two weeks or so.

The counting will be done using the Condorcet method.

Refs: #1104 (comment)

Bugs should be sent to: https://github.com/stduhpf/caritat

[Trott]

@aduh95 Is there a public source code repository for the tool? It seems to be https://github.com/stduhpf/caritat but with some additional files. Maybe there's a tag/branch/fork I should be looking at? I did the tsc compilation step, but I still don't have (for example) app.js.

[aduh95]

@aduh95 Is there a public source code repository for the tool? It seems to be https://github.com/stduhpf/caritat but with some additional files. Maybe there's a tag/branch/fork I should be looking at? I did the tsc compilation step, but I still don't have (for example) app.js.

Yes, it is https://github.com/stduhpf/caritat. app.js has been deleted, it was a left-over from a not-so-clean local repo. I've published v0.2.0 that fixes that, repo and npm package should be in sync now.

[Trott]

Yes, it is https://github.com/stduhpf/caritat. app.js has been deleted, it was a left-over from a not-so-clean local repo. I've published v0.2.0 that fixes that, repo and npm package should be in sync now.

Awesome, thanks.

[MylesBorins]

this is very very cool. Will dig more into implementation in a bit

[mhdawson]

@aduh95 its not immediately obvious if we put higher scores for options we want my guess is that if I wanted to rank them from highest preference to lowest I could use 0-5 for the scores where 5 is the one I prefer the most? Can you confirm that is correct?

[aduh95]

@aduh95 its not immediately obvious if we put higher scores for options we want my guess is that if I wanted to rank them from highest preference to lowest I could use 0-5 for the scores where 5 is the one I prefer the most? Can you confirm that is correct?

@mhdawson that would works, absolutely. As long you put integers, you can use whichever scale that's more convenient for you (negative numbers also works btw): the largest score for the option(s) you prefer the most (5 in your example), the lowest score for the option(s) you prefer the less (0 in your example).

[mhdawson]

@aduh95 thanks

[aduh95]

@nodejs/tsc Reminder that there's an ongoing vote on this PR, you are invited to participate; see the OP for instructions, don't hesitate to ask questions if you have any.
Vote is closing next week if we have quorum.

[aduh95]

@jasnell you mentioned on the issue you don't have a preference but there's one option you don't like. FWIW you can cast a vote that set all the options to 0 (i.e. abstain) except for the issue you don't like that you can set to -1, that will be taken into account when counting the ballots.

[RaisinTen]

Running into an EACCES when I'm trying to vote using sh - it commits an empty file for me. Reported to nodejs/caritat#3.

[RaisinTen]

I used the web ui to generate the encrypted ballot this time and after pushing I realize that it doesn't include the author info (reported in nodejs/caritat#5). Adding it manually.

[tniessen]

Use primordials in the error path only if there are no significant perf regression.

I am confused by the winning option. Does it explicitly support primordials? In other words, could this option be rephrased as Use primordials in the error path unless there are significant perf regressions but not as Do not use primordials in the error path if there are significant perf regressions?

[RaisinTen]

To my understanding, it means - if someone volunteers to add primordials to error paths, we are likely to accept it if and only if there are no significant perf regressions.

[aduh95]

And the reciprocal would be – if someone opens a PR to remove primordials from error paths, it would be rejected, unless said primordials was causing a measurable perf regression.
If we do other votes related to primordials in the future, I agree we should try to make that wording less confusing.

[tniessen]

we are likely to accept it if and only if there are no significant perf regressions.

Does that mean writing benchmarks for error paths?

[Trott]

Given that this vote has been completed, should this be moved out of draft mode and landed?

[aduh95]

Given that this vote has been completed, should this be moved out of draft mode and landed?

The next step would be for me (or anyone that wants to beat me to it) to decrypt all the ballots, then I think it can be merged. Maybe we should also create a .json file similar to 2021-09-29-0.json so participation in this vote can be counted by the automation.

[RaisinTen]

Does that mean writing benchmarks for error paths?

I don't think there is any other way to make sure that the performance is not degraded. So IIUC, PRs like nodejs/node#41702 that attempt to add primordials to an error path would be blocked because we still don't have benchmarks testing the error path.

[ljharb]

I’m confused; what about code not in the error path?

[tniessen]

Does that mean writing benchmarks for error paths?

I don't think there is any other way to make sure that the performance is not degraded.

That's... interesting.

I’m confused; what about code not in the error path?

That was out of scope for this vote... I believe the intention is for more votes to follow.

[targos]

Does that mean writing benchmarks for error paths?

I don't think there is any other way to make sure that the performance is not degraded.

That's... interesting.

Interesting indeed. I thought that we were voting on the error paths because performance regressions that only happen there are not problematic.
My understanding was that we would only block a change in an error path if that somehow affects the performance of non-error paths.

[Trott]

Concurring with @targos and @tniessen in that I don't think we've ever been terribly concerned about error-path performance. I could be mistaken, but it seems like an odd place to put in extra effort on performance.

[mhdawson]

LGTM

[mhdawson]

Concurring with @targos and @tniessen in that I don't think we've ever been terribly concerned about error-path performance. I could be mistaken, but it seems like an odd place to put in extra effort on performance.

For me I found it a bit difficult to be voting about the error path without having discussed if we should use for the main paths. As such I voted more conservatively than I might otherwise have.

[RaisinTen]

LGTM

[BridgeAR]

I agree that we have not been concerned about the error cases performance a lot and as such it's probably fine. I just wonder why we added that option to our vote if we do not plan to actually handle it different from "Make the code in the error path full tamper proof by using primordials" as it seems like the overall vote would have been different in that case.

[Trott]

Tested this with the tooling and it worked great. Thanks.