Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/df78e305-93b0-4605-b9bc-451845689bfc Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
…malizeLeadingWhitespace to pkg/stringutil Agent-Logs-Url: https://github.com/github/gh-aw/sessions/df78e305-93b0-4605-b9bc-451845689bfc Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
|
Hey One thing worth tidying up before merge:
If you'd like a hand cleaning this up, here's a ready-to-run prompt:
|
There was a problem hiding this comment.
Pull request overview
Refactors duplicate/general-purpose string helpers into pkg/stringutil, updates workflow call sites, and removes one duplicate experiment slice parser. The diff also includes some unrelated docs/workflow metadata edits outside the stated refactor scope.
Changes:
- Moved
formatListandnormalizeLeadingWhitespaceintopkg/stringutiland updated workflow usages/tests. - Replaced duplicate
extractStringSlicecalls in experiment parsing with the canonicalparseStringSliceAny. - Also changed OTLP reference docs, agent-factory status docs, and one generated workflow lockfile.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/unified_prompt_step.go |
Swaps local whitespace normalization calls to stringutil.NormalizeLeadingWhitespace. |
pkg/workflow/unified_prompt_step_test.go |
Updates the prompt-step test to call the moved helper. |
pkg/workflow/tools_validation_github_toolsets.go |
Replaces local list formatting with stringutil.FormatList in validation errors. |
pkg/workflow/strings.go |
Removes the old private formatList implementation. |
pkg/workflow/compiler_experiments.go |
Reuses parseStringSliceAny instead of the deleted duplicate slice parser. |
pkg/stringutil/stringutil.go |
Adds exported FormatList and NormalizeLeadingWhitespace. |
pkg/stringutil/stringutil_test.go |
Adds direct tests for the promoted string utilities. |
docs/src/content/docs/reference/frontmatter-full.md |
Expands OTLP endpoint reference docs; appears unrelated to the refactor. |
docs/src/content/docs/agent-factory-status.mdx |
Adds three agent-factory table rows; appears unrelated to the refactor. |
.github/workflows/metrics-collector.lock.yml |
Adds one workflow env version line in a generated lockfile. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 10/10 changed files
- Comments generated: 3
| // Find minimum leading whitespace (excluding empty lines) | ||
| minLeadingSpaces := -1 | ||
| for _, line := range lines { | ||
| if strings.TrimSpace(line) == "" { | ||
| continue // Skip empty lines | ||
| } | ||
| leadingSpaces := len(line) - len(strings.TrimLeft(line, " ")) |
| # OTLP endpoint configuration. Accepts a plain URL string (backward-compat), a | ||
| # single {url, headers} object, or an array of {url, headers} objects for | ||
| # multi-endpoint concurrent fan-out. Encoded as GH_AW_OTLP_ENDPOINTS (JSON array). | ||
| # (optional) | ||
| # This field supports multiple formats (oneOf): | ||
|
|
||
| # Option 1: OTLP collector endpoint URL (e.g. 'https://traces.example.com:4317'). | ||
| # Supports GitHub Actions expressions such as ${{ secrets.OTLP_ENDPOINT }}. When a | ||
| # static URL is provided, its hostname is automatically added to the network | ||
| # firewall allowlist. | ||
| endpoint: "example-value" | ||
|
|
||
| # Option 2: A single OTLP endpoint with a URL and optional per-endpoint headers. | ||
| endpoint: | ||
| # OTLP collector endpoint URL (e.g. 'https://traces.example.com:4317'). Supports | ||
| # GitHub Actions expressions such as ${{ secrets.OTLP_ENDPOINT }}. When a static | ||
| # URL is provided, its hostname is automatically added to the network firewall | ||
| # allowlist. | ||
| url: "example-value" | ||
|
|
||
| # (optional) | ||
| # This field supports multiple formats (oneOf): | ||
|
|
||
| # Option 1: Map of HTTP header names to values. Values support GitHub Actions | ||
| # expressions such as ${{ secrets.TOKEN }}. | ||
| headers: | ||
| {} | ||
|
|
||
| # Option 2: Deprecated: use the map form instead. Comma-separated list of | ||
| # key=value HTTP headers (e.g. 'Authorization=Bearer <token>'). Supports GitHub | ||
| # Actions expressions such as ${{ secrets.OTLP_HEADERS }}. | ||
| headers: "example-value" | ||
|
|
||
| # Option 3: Multiple OTLP collector endpoints to export traces to concurrently. | ||
| # Each entry has its own URL and optional per-endpoint headers. | ||
| endpoint: [] | ||
| # Array items: A single OTLP endpoint with a URL and optional per-endpoint | ||
| # headers. | ||
|
|
||
| # HTTP headers for the backward-compat string endpoint form. Only used when | ||
| # endpoint is a plain string; object/array endpoint entries carry their own | ||
| # per-endpoint headers. |
| | [Daily File Diet](https://github.com/github/gh-aw/blob/main/.github/workflows/daily-file-diet.md) | copilot | [](https://github.com/github/gh-aw/actions/workflows/daily-file-diet.lock.yml) | `daily around 13:00 on weekdays` | - | | ||
| | [Daily Firewall Logs Collector and Reporter](https://github.com/github/gh-aw/blob/main/.github/workflows/daily-firewall-report.md) | copilot | [](https://github.com/github/gh-aw/actions/workflows/daily-firewall-report.lock.yml) | - | - | | ||
| | [Daily Go Function Namer](https://github.com/github/gh-aw/blob/main/.github/workflows/daily-function-namer.md) | claude | [](https://github.com/github/gh-aw/actions/workflows/daily-function-namer.lock.yml) | - | - | | ||
| | [Daily Grafana OTel Instrumentation Advisor](https://github.com/github/gh-aw/blob/main/.github/workflows/daily-grafana-otel-instrumentation-advisor.md) | claude | [](https://github.com/github/gh-aw/actions/workflows/daily-grafana-otel-instrumentation-advisor.lock.yml) | - | - | |
|
@copilot review all comments |
…elated doc/lockfile changes Agent-Logs-Url: https://github.com/github/gh-aw/sessions/27539f97-fb71-4d10-a2de-fb1a279cd0a9 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed in 9a7a99e:
|
Addresses three findings from semantic function clustering analysis: one confirmed near-duplicate, two general-purpose utilities misplaced in the
workflowpackage.Changes
Remove
extractStringSliceduplicate (compiler_experiments.go)extractStringSlice(raw)calls withparseStringSliceAny(raw, nil)— the canonical version already inparse_helpers.goextractStringSlice; the canonical version also handlesnilinput and pre-allocates the result slicePromote
formatList→stringutil.FormatListworkflow/strings.goto exportedpkg/stringutil/stringutil.go"a and b, and c"instead of"a, b, and c"; replaced recursion withstrings.Join:tools_validation_github_toolsets.goto usestringutil.FormatListPromote
normalizeLeadingWhitespace→stringutil.NormalizeLeadingWhitespaceunified_prompt_step.goto exportedpkg/stringutil/stringutil.gostrings.TrimLeft(line, " \t")instead ofstrings.TrimLeft(line, " ")so that tab-indented and mixed tab/space indented content is correctly dedentedunified_prompt_step.goand the existing testTests
TestFormatListandTestNormalizeLeadingWhitespacetopkg/stringutil/stringutil_test.go, including tab-indentation and mixed-indentation test casesNo change needed
close_entity_helpers.goalready contains the architectural comment explaining the registry-vs-per-file choice