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

fix: use case-insensitive string comparison for fast-track approvals #658

Merged
merged 1 commit into from
Nov 8, 2022

Conversation

Trott
Copy link
Member

@Trott Trott commented Nov 8, 2022

I noticed today that a PR that had sufficient fast-track approvals was being rejected as needing one more approval. This is because one of the approvers' had a GitHub handle where the case in the README (linkgoron) did not match the case returned by GitHub (Linkgoron). Since GitHub handles are case-insensitive, this change makes the comparison case-insensitive.

One might be tempted to think that we need to do the same for the list of TSC members, but handles are never compared there so it is not necessary.

I noticed today that a PR that had sufficient fast-track approvals was
being rejected as needing one more approval. This is because one of the
approvers' had a GitHub handle where the case in the README
(`linkgoron`) did not match the case returned by GitHub (`Linkgoron`).
Since GitHub handles are case-insensitive, this change makes the
comparison case-insensitive.

One might be tempted to think that we need to do the same for the list
of TSC members, but handles are never compared there so it is not
necessary.
@codecov
Copy link

codecov bot commented Nov 8, 2022

Codecov Report

Base: 83.60% // Head: 83.60% // No change to project coverage 👍

Coverage data is based on head (754e325) compared to base (beda17c).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #658   +/-   ##
=======================================
  Coverage   83.60%   83.60%           
=======================================
  Files          37       37           
  Lines        4117     4117           
=======================================
  Hits         3442     3442           
  Misses        675      675           
Impacted Files Coverage Δ
lib/pr_checker.js 98.44% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@aduh95 aduh95 merged commit 8ad4b37 into nodejs:main Nov 8, 2022
@Trott Trott deleted the case branch November 8, 2022 14:41
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

Successfully merging this pull request may close these issues.

4 participants