Skip to content

chore(ci): exclude email templates and load-tests from SonarCloud#385

Merged
thomasluizon merged 2 commits into
mainfrom
chore/api-sonar-exclude-generated
Jul 13, 2026
Merged

chore(ci): exclude email templates and load-tests from SonarCloud#385
thomasluizon merged 2 commits into
mainfrom
chore/api-sonar-exclude-generated

Conversation

@thomasluizon

Copy link
Copy Markdown
Owner

What

Append two patterns to sonar.exclusions in the SonarScanner Begin step of .github/workflows/sonarcloud.yml, so it becomes:

**/Migrations/**,**/bin/**,**/obj/**,**/OAuth/OAuthLoginPage.cs,**/.claude/**,**/Email/Templates/**,**/load-tests/**

Why

Config-only change that clears ~80 false-positive / non-product SonarCloud smells. Verified: every open Web:* smell is under src/Orbit.Infrastructure/Email/Templates/*.html and every open javascript:* smell is under load-tests/.

  • **/Email/Templates/** — HTML-email templates. Sonar's Web:S1827 (remove cellspacing), Web:S6819 (use <img> not role="presentation") and Web:S5257 (replace layout table with CSS) are false positives for email: table-based layout, cellspacing/cellpadding, and role="presentation" are the required, intentional cross-client techniques for HTML email (Outlook/legacy clients don't support CSS layout; every email framework emits exactly this). These are static assets with no maintainability value to analyze.
  • **/load-tests/** — k6 load-test scenario scripts. Developer tooling, not shipped product code — parity with the existing **/.claude/** exclusion. Clears the open javascript:* smells.

No product code is excluded and no application source changed, so no build impact.

Validation

Because the workflow runs on pull_request from the PR head, this PR's own SonarCloud scan re-runs with the new exclusions — the ~72 Web:* + ~8 javascript:* smells drop to 0 with no new issues.

Refs thomasluizon/orbit-ui-mobile#243

Append two entries to sonar.exclusions in the SonarScanner Begin step:

- **/Email/Templates/**: HTML-email templates. Web:S1827 (cellspacing),
  Web:S6819 (role=presentation over <img>) and Web:S5257 (layout tables)
  are false positives here — table layout, cellspacing/cellpadding and
  role="presentation" are the required cross-client techniques for HTML
  email (Outlook/legacy clients lack CSS-layout support). Static assets
  with no maintainability value to analyze.
- **/load-tests/**: k6 load-test scenario scripts — developer tooling,
  not shipped product code (parity with the existing **/.claude/**
  exclusion). Clears the open javascript:* smells under load-tests/.

Refs thomasluizon/orbit-ui-mobile#243

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review: PR #385 — chore(ci): exclude email templates and load-tests from SonarCloud

Scope: PR #385 in thomasluizon/orbit-api
Recommendation: APPROVE

Summary

Single-file, config-only change to .github/workflows/sonarcloud.yml: appends **/Email/Templates/** and **/load-tests/** to the SonarScanner Begin step's sonar.exclusions, plus a documentation comment explaining why. Verified both directories exist as claimed and contain only non-product assets (HTML/text email templates, k6 JS load-test scripts) with zero .cs files and no .csproj/.slnx references, so no product source is excluded and no build/test surface changes.

Findings

Critical

None.

High

None.

Medium

None.

Low / Info

None (nothing worth surfacing per the signal gate).

Subagents

Agent Verdict
security-reviewer N/A — gate not met: diff touches zero src/ files (CI workflow config only). Manually confirmed the change only widens sonar.exclusions (a static-analysis scope, not a security control) and does not touch permissions:, secrets handling, or checkout/build steps.
contract-aligner N/A — gate not met: no DTO, Controller route, or packages/shared type/endpoints.ts changed.

Validation

Check Result
Build (dotnet) N/A — CI adaptation: Build runs as a separate required check.
Tests (dotnet) N/A — CI adaptation: Unit Tests run as a separate required check.
YAML well-formed Manually inspected full file; indentation/structure consistent with the rest of the step, no syntax break.

Deferred — N/A dimensions & files not verdicted

  • Dead/stale code, SOLID/clean-arch, No-workaround, Type safety, No console.log — N/A, no application code changed.
  • DESIGN.md/AI-slop, Parity (#9), i18n (#10) — N/A, no apps/* UI files touched; these are frontend-only dimensions and not applicable to an orbit-api workflow change.
  • Backend hard rules (#13) — N/A, the only changed file is .github/workflows/sonarcloud.yml; no C# code, so timezone/authz/validation/logging/transaction/test rules have no applicable surface.
  • FEATURES.md parity (#14) — N/A, no user-facing feature surface changed (CI tooling only, invisible to end users).
  • Contract drift + backward-compat (#11) — N/A, no DTO/schema/endpoint changed; also not verifiable in CI (sibling orbit-ui-mobile repo not checked out here), though moot since nothing to compare.
  • Every changed file (1 of 1: .github/workflows/sonarcloud.yml) received a verdict above — nothing left uncovered.

What's good

  • The PR body's technical justification is accurate and independently verified: the cited Sonar rules (Web:S1827, Web:S6819, Web:S5257) are maintainability/accessibility rules about HTML table layout, not security rules, so excluding them from analysis of intentionally table-based HTML emails doesn't blind Sonar to a real defect class.
  • Both excluded globs were checked against the actual tree: src/Orbit.Infrastructure/Email/Templates/ contains only .html/.txt template assets, and load-tests/ contains only k6 .js scenario scripts plus a README — neither is referenced by any .csproj/.slnx, so nothing compiled or test-covered is excluded.
  • The new comment follows the existing block's established convention (added in the #243 series) of a one-line-per-entry WHY explanation, keeping the file self-documenting for future maintainers.
  • Scope is minimal and precisely targeted — no unrelated exclusions, no widening beyond the two directories named in the PR description.

Recommendation

Approve as-is. No follow-up required.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review: PR #385

Scope: PR #385 in thomasluizon/orbit-api — chore(ci): exclude email templates and load-tests from SonarCloud
Recommendation: APPROVE

Summary

Single-file, config-only change to .github/workflows/sonarcloud.yml (4 additions/1 deletion): appends **/Email/Templates/** and **/load-tests/** to the existing sonar.exclusions glob list, plus a WHY comment block. No application source, DTO, endpoint, or client-visible surface changed.

Findings

  • Critical: None
  • High: None
  • Medium: None
  • Low/Info: The new comment block mirrors the exact WHY-with-rule-IDs style already used for the pre-existing coverage.exclusions block in the same file — consistent local convention, not narration.

Verification performed

  • src/Orbit.Infrastructure/Email/Templates/ contains only .html/.txt template assets — the actual renderer logic (EmailTemplateRenderer.cs, EmailCopy.cs) and its unit test (tests/Orbit.Infrastructure.Tests/Email/EmailTemplateRendererTests.cs) live outside that folder and remain scanned/covered by the new glob (**/Email/Templates/** doesn't reach them).
  • load-tests/ is a single top-level directory (k6 README.md, lib/, results/, scenarios/) with no overlap with any src/ path — no product code excluded.
  • YAML additions are correctly comma-separated, same quoting/format as the existing entries.

Subagents

  • security-reviewer: gate (src/ code changed) not met — not fired. Manually checked: the new globs only exclude non-code static assets from smell-scanning, not from build/test/coverage; no auth/security-relevant code is touched.
  • contract-aligner: gate (DTO/route/shared-type change) not met — not fired.

Deferred

Dimensions 2, 3, 6–11, 13, 14 are N/A — the diff touches no application code, UI, contract surface, or user-facing feature.

What's good

Each exclusion is backed by cited Sonar rule IDs and a concrete technical rationale (HTML-email table-layout requirement, k6 dev-tooling parity with the existing **/.claude/** exclusion); scope is narrow, no product code is excluded.

@sonarqubecloud

Copy link
Copy Markdown

@thomasluizon
thomasluizon merged commit 504f8b8 into main Jul 13, 2026
19 checks passed
@thomasluizon
thomasluizon deleted the chore/api-sonar-exclude-generated branch July 13, 2026 16:09
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.

1 participant