Fix CA1707 test-naming warnings and GitHub Actions Node 20 deprecation - #30
Merged
Merged
Conversation
Two independent sources of noisy CI warnings:
- CA1707 ("remove underscores from member name") was firing on every
test method in MainFormMultiTabTests.cs despite the repo already
having a [JianpuEditor.Tests/**/*.cs] .editorconfig section that
suppresses it for test naming. The glob has a real quirk: "**/*.cs"
only matches files inside a subdirectory, not files sitting directly
in JianpuEditor.Tests/ itself -- which is exactly where
MainFormMultiTabTests.cs lives (Glue/DocumentTabTests.cs, one level
down, was already covered, which is why this went unnoticed).
Widened the glob to "{*.cs,**/*.cs}" to cover both cases. Confirmed
with a clean rebuild of the whole solution: 0 warnings, 0 errors
(previously 10-11 CA1707 warnings from this one file).
- The four pinned GitHub Actions (checkout, setup-dotnet, cache,
upload-artifact) are still on major versions whose action.yml
declares node20, which GitHub now runs on node24 anyway but warns
about every build ahead of node20's removal from runners. Bumped each
to the latest major version that declares node24 in its own
action.yml (verified by fetching the actual action.yml at each tag,
not just release notes): checkout v4 -> v7, setup-dotnet v4 -> v6,
cache v4 -> v6, upload-artifact v4 -> v7, in both ci.yml and
release.yml. Checked each for breaking changes relevant to this
repo's usage (checkout v7's fork-PR restriction only affects
pull_request_target/workflow_run triggers, which this repo doesn't
use; the others are internal Node/ESM migrations with no input/output
changes affecting the options already in use here).
Verified via the same sandbox pipeline as prior changes: a clean
`dotnet build` of the whole solution (temporarily cross-compiling
JianpuEditor.Tests with EnableWindowsTargeting, reverted, not
committed) now produces 0 warnings/0 errors, and dotnet format
--verify-no-changes passes for both projects. The GitHub Actions
version bumps themselves can't be executed in this sandbox (no GHA
runner here) -- verified by fetching each new tag's actual action.yml
to confirm node24 and reading each release's changelog for breaking
changes, but the real test is this branch's next CI run.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pguj4XSScE141p1ScWoqEr
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two independent sources of noisy CI warnings:
MainFormMultiTabTests.csdespite the repo already having a[JianpuEditor.Tests/**/*.cs].editorconfigsection that suppresses it for test naming. The glob has a real quirk:**/*.csonly matches files inside a subdirectory, not files sitting directly inJianpuEditor.Tests/itself — which is exactly whereMainFormMultiTabTests.cslives(
Glue/DocumentTabTests.cs, one level down, was already covered, which is why this wentunnoticed). Widened the glob to
{*.cs,**/*.cs}to cover both cases.checkout,setup-dotnet,cache,upload-artifact) arestill on major versions whose
action.ymldeclaresnode20, which GitHub now runs onnode24anyway but warns about every build ahead of
node20's removal from runners. Bumped each to thelatest major version that declares
node24in its ownaction.yml(verified by fetching theactual
action.ymlat each tag, not just release notes):checkoutv4 → v7,setup-dotnetv4→ v6,
cachev4 → v6,upload-artifactv4 → v7, in bothci.ymlandrelease.yml.Test plan
dotnet buildof the whole solution (temporarily cross-compilingJianpuEditor.Testswith
EnableWindowsTargeting, reverted, not committed) now produces 0 warnings, 0 errors(previously 10–11 CA1707 warnings from
MainFormMultiTabTests.csalone).dotnet format --verify-no-changespasses for both projects.checkoutv7'sfork-PR restriction only affects
pull_request_target/workflow_runtriggers, which this repodoesn't use; the others are internal Node/ESM migrations with no input/output changes affecting
the options already in use here.
Not verified in this sandbox: the GitHub Actions version bumps themselves can't be executed
here (no GHA runner) — the real test is this PR's own CI run, which I'm watching.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Pguj4XSScE141p1ScWoqEr
Generated by Claude Code