Skip to content

Conversation

@GaneshPatil7517
Copy link

@GaneshPatil7517 GaneshPatil7517 commented Nov 12, 2025

"I've opened a PR that fixes the voting overview bug by removing former TSC members from the tracking file and normalizing newly added members: . Please review when convenient; happy to adjust to archive removed members instead of deleting them."

Summary by CodeRabbit

  • Bug Fixes

    • Fixed typo in error message.
  • Improvements

    • Implemented automatic normalization of vote data including counts and timestamps with sensible defaults.
    • Enhanced member filtering to ensure vote tracking includes only current TSC members.
    • Optimized vote record updates to single atomic write operation for improved reliability and consistency.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@coderabbitai
Copy link

coderabbitai bot commented Nov 12, 2025

Walkthrough

Vote tracking script refactored to normalize and augment vote data, merge newly discovered TSC members, and filter entries against current membership. Data transformations include type coercion for counts and timestamps, boolean conversion for participation flags, and default value assignments. Vote details written in single operation after processing.

Changes

Cohort / File(s) Change Summary
Vote Tracker Data Normalization & Filtering
\.github/scripts/vote_tracker/index.js
Merges newly discovered TSC members unconditionally; normalizes vote details (coerces counts to numbers, converts boolean flags, sets defaults for dates); filters entries to retain only those matching current TSC members (case-insensitive); consolidates write operation to single file write; corrects typo "wile" to "while"; computes lowercased TSC member set and current date within update flow.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify normalization logic for each field (counts, timestamps, boolean flags) handles edge cases and null/undefined values correctly
  • Ensure case-insensitive filtering against TSC members operates as intended
  • Confirm default value assignments (zeros, false, today's date) meet requirements
  • Validate the consolidated write operation doesn't introduce race conditions or data loss compared to previous per-branch flow

Possibly related PRs

Suggested reviewers

  • derberg
  • bandantonio
  • thulieblack

Poem

🐰 A hop through data, fresh and bright,
TSC members sorted just right,
Counts and flags all normalized true,
One write, one file, our work is through!
While typos flee, the tracker runs free,
Vote tracking magic—let's agree!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: removing former TSC members from voteTrackingFile, which aligns with the primary objective and the significant filtering logic added.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5665dac and 2abb219.

📒 Files selected for processing (1)
  • .github/scripts/vote_tracker/index.js (1 hunks)

Comment on lines +264 to +266
if (typeof e.isVotedInLast3Months !== "boolean") {
e.isVotedInLast3Months = Boolean(e.isVotedInLast3Months);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fix boolean coercion for legacy string values.

Boolean(e.isVotedInLast3Months) flips any non-empty string—including "false", "no", or "0"—to true, which marks inactive members as active voters and corrupts the participation stats you’re trying to normalize. Please normalize string inputs explicitly before coercing.

-        if (typeof e.isVotedInLast3Months !== "boolean") {
-          e.isVotedInLast3Months = Boolean(e.isVotedInLast3Months);
-        }
+        if (typeof e.isVotedInLast3Months !== "boolean") {
+          const normalized = String(e.isVotedInLast3Months).trim().toLowerCase();
+          e.isVotedInLast3Months = !["", "false", "0", "no", "off"].includes(normalized);
+        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (typeof e.isVotedInLast3Months !== "boolean") {
e.isVotedInLast3Months = Boolean(e.isVotedInLast3Months);
}
if (typeof e.isVotedInLast3Months !== "boolean") {
const normalized = String(e.isVotedInLast3Months).trim().toLowerCase();
e.isVotedInLast3Months = !["", "false", "0", "no", "off"].includes(normalized);
}
🤖 Prompt for AI Agents
In .github/scripts/vote_tracker/index.js around lines 264-266, the code uses
Boolean(e.isVotedInLast3Months) which treats any non-empty string (including
"false", "no", "0") as true; replace this with explicit normalization: if the
value is already a boolean leave it, if it's a string trim and toLowerCase() and
return true only for accepted truthy strings like "true", "1", "yes" (and false
for "false", "0", "no" or unknown), and also handle numeric 1/0 by mapping 1 ->
true and 0 -> false; set e.isVotedInLast3Months to the normalized boolean.

@thulieblack
Copy link
Member

I don't see the issue that is related to this PR ?

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.

2 participants