ci(publish): grep -qFx + parenthesize publish if - #87
Conversation
Add .github/copilot-instructions.md scoping Copilot Code Review to the five-package shape: netstandard2.0 surface constraints (no System.Text.Json or modern BCL in main lib / Sources), Sources visibility-rewrite invariant (#if ANCPLUA_ROSLYN_PUBLIC), generator hot-path patterns (ref structs, EquatableArray<T>, closure-free dict ops), Guard.cs as the sole RS1035 allow-list site. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tion if Two zero-risk readability/robustness tweaks on the auto-bump workflow: - grep -qFx (literal-string match) instead of grep -qx — avoids regex metachar surprises if a tag ever contains a literal . or * - (push && main) || dispatch — parens make the precedence explicit (no behavior change; YAML expression already evaluated in this order) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📜 Recent review details⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🧰 Additional context used📓 Path-based instructions (1).github/**⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (1)📓 Common learnings🔇 Additional comments (3)
Summary by CodeRabbit
Note: This release contains no user-facing changes. WalkthroughTwo independent infrastructure changes: a new Copilot review guidance document for PR reviews, and minor NuGet publish workflow refinements to version detection and job gating. ChangesCopilot PR Review Instructions
NuGet Publish Workflow
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Review rate limit: 4/5 reviews remaining, refill in 12 minutes. Comment |
There was a problem hiding this comment.
Pull request overview
This PR makes two small, defensive updates to the NuGet publish workflow and adds repository-specific Copilot PR review guidance. In the broader codebase, the workflow change improves clarity and matching robustness in the auto-publish path introduced in #86.
Changes:
- Switched
grep -qxtogrep -qFxwhen checking whetherHEADalready has the latest tag, avoiding unintended regex interpretation. - Parenthesized the
publishjob condition to make the intended(push && main) || dispatchprecedence explicit. - Added
.github/copilot-instructions.mdwith repository-specific PR review guidance and project context.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
.github/workflows/nuget-publish.yml |
Tightens tag matching and clarifies the publish-job condition without changing intended workflow behavior. |
.github/copilot-instructions.md |
Adds repo-specific Copilot review instructions, package/context notes, and review heuristics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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 adds a .github/copilot-instructions.md file containing project-specific guidelines for AI code reviews. The feedback identifies a mismatch between the PR metadata and the actual changes, a versioning error regarding .NET 10, and technical inaccuracies in the descriptions of Span<T> and EquatableArray<T>.
| @@ -0,0 +1,68 @@ | |||
| # Copilot PR-review instructions for ANcpLua.Roslyn.Utilities | |||
There was a problem hiding this comment.
The Pull Request title and description do not match the changes provided in this patch. The title and summary describe CI workflow updates (specifically grep flags and YAML expression parentheses), but the actual change is the addition of a Copilot instructions markdown file. This discrepancy should be resolved to ensure the PR metadata accurately reflects its contents.
| package (rewritten to `internal` at pack via `Transform-Sources.ps1`), `Polyfills`, | ||
| `Testing` (analyzer/generator/codefix harness), and `Testing.Aot`. Main library + | ||
| `Sources` target `netstandard2.0` so Roslyn-hosted consumers can absorb them; | ||
| `Testing` targets `net10.0`. `TreatWarningsAsErrors=true` repo-wide; all analyzer |
There was a problem hiding this comment.
The text refers to net10.0 as a target framework. As of the current release cycle, .NET 9 is the latest version, and .NET 10 is not yet available. This is likely a typo for net8.0 or net9.0. This also appears on line 60.
| - Storing `ISymbol`, `SyntaxNode`, `SyntaxTree`, or any Roslyn reference type as | ||
| a `HashSet<T>` / `Dictionary<TKey, …>` key — reference equality, cache misses | ||
| on rebuild, breaks generator incrementality. | ||
| - New allocations in generator hot paths: `.ToArray()` / `.ToList()` on a `Span` |
There was a problem hiding this comment.
The Span<T> type does not have a .ToList() extension method in the standard .NET BCL. If the intent is to flag heap allocations from converting a span to a collection, .ToArray() is the correct method. If you intended to cover general collection allocations, consider clarifying the types involved.
| - New allocations in generator hot paths: `.ToArray()` / `.ToList()` on a `Span` | |
| - New allocations in generator hot paths: .ToArray() on a Span |
| the main library. | ||
| - `Testing` ships a fluent generator/analyzer/codefix harness. New test helpers | ||
| belong there, not in individual consumer test projects. | ||
| - `EquatableArray<T>` is `ref`-struct-like (value-equality wrapper); use it for |
There was a problem hiding this comment.
EquatableArray<T> is a regular readonly struct, not a ref struct. Describing it as "ref-struct-like" is technically inaccurate and potentially misleading for an AI reviewer, especially since ref struct types cannot be used as fields in records (which the following line explicitly recommends). It is better described as a "value-type wrapper" or as having "value semantics".
| - `EquatableArray<T>` is `ref`-struct-like (value-equality wrapper); use it for | |
| - EquatableArray<T> is a value-type wrapper (providing value equality); use it for |
Summary
Two zero-risk readability/robustness tweaks on the auto-bump workflow merged in #86:
Test plan
🤖 Generated with Claude Code