feat: implement issue #428 — SonarCloud: JavaScript/TypeScript code quality - #438
feat: implement issue #428 — SonarCloud: JavaScript/TypeScript code quality#438don-petry wants to merge 9 commits into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR applies JavaScript quality-rule updates across deployment, installer, calendar, Gmail, and utility code. It adds SonarCloud coverage generation and LCOV configuration, improves UI test synchronization, updates test mocks, and adjusts two workflow template output blocks. ChangesCode quality and coverage updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
CI Failure: SonarCloud Code AnalysisStep: Quality Gate check SonarCloud's Quality Gate failed because 0.0% Coverage on New Code was reported, against a required threshold of ≥ 80%. The PR modifies several JS files ( Suggested fix: Add or extend unit tests (e.g. in |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modernizes the codebase by adopting newer JavaScript features, such as optional chaining (?.), Date.now(), Number.parseInt, and node: prefixed built-in imports. It also simplifies several regular expressions, replaces a hardcoded timeout in UI tests with element visibility assertions, and updates error types to TypeError where appropriate. The review feedback correctly points out that using replaceAll with a global regular expression is redundant and potentially risky compared to replace, suggesting a revert for those specific changes.
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
Dev-Lead — fix-reviews (applied)Changes committed and pushed. |
There was a problem hiding this comment.
Pull request overview
This PR addresses SonarCloud JavaScript/TypeScript code-quality findings across the repo by applying mostly mechanical refactors (optional chaining/nullish coalescing, node: core imports, replaceAll, Number.parseInt, Date.now(), regex simplifications, and small test refactors) while aiming to keep runtime behavior unchanged.
Changes:
- Modernize JS patterns to satisfy Sonar rules (optional chaining,
??,replaceAll,Number.parseInt,Date.now(),node:imports, simplified regexes). - Improve test quality/readability (parameterized Jest test; replace fixed Playwright sleep with an observable UI condition).
- Tighten type-check error signaling in deploy tooling (
TypeErrorfor type validation).
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test-utils/setup.js | Switch to require('node:crypto') for core module import. |
| test-utils/mocks.js | Use optional chaining to simplify safe property/method access in mocks. |
| src/gmail-to-drive-by-labels/tests/code.test.js | Update Date mocking to align with Date.now() usage. |
| src/gmail-to-drive-by-labels/src/index.js | Apply Sonar-driven refactors (` |
| src/gas-utils.js | Apply Sonar-driven refactors (regex changes, replaceAll, optional chaining, node:crypto). |
| src/deploy/index.js | Use TypeError for type-check failures. |
| src/calendar-to-sheets/src/index.js | Split sort().forEach() into separate statements to satisfy rule guidance. |
| src/calendar-to-briefing-doc/tests/index.test.js | Replace repetitive tests with it.each parameterized test. |
| src/calendar-to-briefing-doc/src/index.js | Use optional chaining and ?? for clearer defaults/guards. |
| scripts/check-coverage.js | Use node: imports and a for...of loop for branch coverage iteration. |
| gas-installer/src/index.js | Prefer endsWith() and optional chaining in filtering. |
| gas-installer/Index.html | Prefer replaceAll() for HTML escaping helpers. |
| deploy/tests/ui.spec.js | Use node:path and replace fixed waits with DOM visibility synchronization. |
| deploy/index.html | Apply Sonar-driven refactors (remove unused var, regex simplification, optional chaining, Number.parseInt, replaceAll, simplify template usage). |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
CI Failure: SonarCloud Code AnalysisStep: Quality Gate The Quality Gate failed on a single condition: 0.0% Coverage on New Code, against a required minimum of 80%. This PR changes logic in Suggested fix: Add or extend test cases in |
Dev-Lead — review-changes (applied)Changes committed and pushed. |
CI Failure: SonarCloud Code AnalysisStep: Quality Gate The SonarCloud Quality Gate failed with 0.0% Coverage on New Code (required ≥ 80%). This PR only touches Suggested fix: Ensure the coverage job actually runs and uploads a report covering the changed files ( |
Dev-Lead Fix CI — appliedPR: #438 | SHA: |
|
Dev-lead assessment: All review threads have been addressed and resolved. The only outstanding CI item is the |
Phase 0 Assessment — CI Failure TriageThe The runner hit a connection reset while downloading the CodeQL bundle ( The failed run (ID No open human review threads remain. All 304 tests pass locally, lint is clean (0 errors). |
|
Dev-Lead Agent — Phase 0 Assessment No open review threads to address. Assessed the one failing CI check:
Local verification:
The PR code is correct and ready. The transient CodeQL failure is an infrastructure flake unrelated to the changes. |
Dev-Lead Agent — Review Response AssessmentOpen human review threads: None to address. CI assessment:
Local verification:
|
|
The |
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #438 |
Dev-Lead — review-changes (applied)Changes committed and pushed. |
Review — fix requested (cycle 1/3)The automated review identified the following issues. Please address each one: Findings to fixAutomated review — NEEDS HUMAN REVIEWRisk: MEDIUM SummaryCode-quality modernization pass driven by SonarCloud findings (issue #428): optional chaining, replaceAll, Number.parseInt, node: import prefixes, Date.now(), regex simplifications, plus SonarCloud workflow hardening (step-scoped SONAR_TOKEN, npm ci --ignore-scripts, coverage upload) and a new SHA-pinned CodeQL workflow. The code changes are behavior-preserving and well-tested, but the PR cannot be approved in its current state: it has merge conflicts with main, the core CI suite never ran on the current head SHA, and the new CodeQL workflow conflicts with the repo's already-enabled CodeQL default setup. Linked issue analysisCloses #428 (SonarCloud: JavaScript/TypeScript code quality — 58 findings, worst severity MAJOR). The diff substantively addresses the flagged rule categories: S6582 (optional chaining), S7781 (replaceAll), S7773 (Number.parseInt), S7772 (node: imports), S5869 (duplicate character-class entries), S3358 (nested ternaries), S7759 (Date.now), S6557 (endsWith). SonarCloud Quality Gate passed on an earlier commit in this PR. The issue is substantively addressed. FindingsBlocking:
Non-blocking notes:
CI statusChecks present on head Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review. Additional tasks
The review cascade will automatically re-review after new commits are pushed. |
|
Closing as part of the open-PR drain (50-PR automation cap). This branch has real merge conflicts with main and cannot be rebased automatically; the linked compliance issue remains OPEN so dev-lead can regenerate a clean PR once there is cap headroom. No work is lost. |
Closes #428
Implemented by dev-lead agent. Please review.
Summary by CodeRabbit
Bug Fixes
Quality Improvements