Skip to content

ci: exclude test projects from the no-docs count - #3295

Merged
ipdae merged 1 commit into
developmentfrom
ci/exclude-test-projects-from-no-docs
Jul 2, 2026
Merged

ci: exclude test projects from the no-docs count#3295
ipdae merged 1 commit into
developmentfrom
ci/exclude-test-projects-from-no-docs

Conversation

@ipdae

@ipdae ipdae commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #3294. The count-no-docs gate counts CS1591/CS1573 across the whole solution, including test projects (~2,900 warnings come from .Lib9c.Tests alone). Because check-items-without-docs-increased compares base vs head counts, this forces every PR that adds a public test class / constructor / [Fact] method to also add XML doc comments to them — pure noise, since test methods are not shippable API.

Change

Filter out warnings attributed to a *.Tests.csproj before counting, so the gate only tracks documentation coverage of the shippable projects:

- | grep -c -E "CS(1573|1591)" || true)
+ | grep -E "CS(1573|1591)" | grep -vc "Tests.csproj" || true)

Applied to both count-no-docs-on-baseref and count-no-docs-on-headref.

Verification (local)

  • Full doc-warning count: 11,153
  • After excluding *.Tests.csproj: 8,206 (the 2,947 excluded are all attributed to test projects)
  • Zero-match case still yields 0 safely (guarded by || true).

CI only — no application code changed.

🤖 Generated with Claude Code

The count-no-docs gate counts CS1591/CS1573 across the whole solution,
including test projects (e.g. ~2900 from .Lib9c.Tests). Because
check-items-without-docs-increased compares base vs head, this forces
every PR that adds a public test class/constructor/[Fact] method to also
add XML doc comments to them, which is noise — test methods are not
public API.

Filter out warnings attributed to a `*.Tests.csproj` before counting so
the gate only tracks documentation coverage of the shippable projects.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ipdae
ipdae merged commit d89752a into development Jul 2, 2026
27 checks passed
ipdae added a commit that referenced this pull request Jul 2, 2026
ArenaCharacter.SetRuneSkills clamped rune skill power to int.MaxValue via
SafeDecimalToInt32, while the PvE Player.SetRuneSkills fix widened it to
long. For a high-stat caster this made PvE and arena compute different
rune skill power (PvE full value, arena capped at ~2.1B). Widen the
active arena SetRuneSkills to long/SafeDecimalToInt64 as well so both
paths agree and match the long-based damage pipeline. The obsolete
ArenaCharacter.SetRuneV1 is left unchanged for historical determinism.

Add an arena regression test alongside the PvE one, and drop the
per-member XML doc comments on the test (no longer needed now that the
count-no-docs gate excludes test projects, #3295).

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