Skip to content

Refactor duplicated update-check state helpers and centralize repo/API normalization#41985

Merged
pelikhan merged 4 commits into
mainfrom
copilot/refactor-util-helpers
Jun 28, 2026
Merged

Refactor duplicated update-check state helpers and centralize repo/API normalization#41985
pelikhan merged 4 commits into
mainfrom
copilot/refactor-util-helpers

Conversation

Copilot AI commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

This refactor removes a few verified helper reinventions and consolidates small but repeated logic in pkg/cli. It also moves the host-aware repo parser to pkg/repoutil, where the rest of the repo string handling already lives.

  • CLI update-check state

    • Extract shared marker-file helpers for:
      • temp-file path creation
      • timestamp writes
      • interval-based gating
    • Reuse the shared helpers from both update_check.go and compile_update_check.go instead of keeping parallel implementations.
  • Repository normalization

    • Move host-aware [HOST/]owner/repo parsing from pkg/cli/outcome_eval.go into pkg/repoutil as NormalizeRepoForAPI.
    • Update CLI call sites that need to split API host from owner/repo.
  • Helper reinventions removed

    • Replace manual permission-key sorting with sliceutil.SortedKeys.
    • Replace local SHA-shortening logic with the existing truncation utility.
    • Co-locate the Actions single-quoted string escaper with the other string escaping helpers in pkg/workflow/strings.go.
  • Tests and package docs

    • Extend repoutil tests/docs to cover the new normalization helper.
    • Keep existing CLI/workflow behavior tests aligned with the helper move.

Example of the repo normalization now living in repoutil:

ownerRepo, host := repoutil.NormalizeRepoForAPI("github.example.com/owner/repo")
// ownerRepo == "owner/repo"
// host      == "github.example.com"

pr-sous-chef run 28311797805

Generated by 👨‍🍳 PR Sous Chef · 111.6 AIC · ⌖ 0.953 AIC · ⊞ 17.2K ·

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor duplicated update-check trio and misplaced repo parser Refactor duplicated update-check state helpers and centralize repo/API normalization Jun 28, 2026
Copilot AI requested a review from pelikhan June 28, 2026 01:37
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

Hey @Copilot 👋 — great work on this refactoring PR! Consolidating the duplicated update-check marker-file helpers and moving NormalizeRepoForAPI into pkg/repoutil where it logically belongs makes the codebase noticeably cleaner. The swaps for sliceutil.SortedKeys and the existing truncation utility are exactly the kind of housekeeping that keeps tech debt in check.

Tests are in place for the new repoutil normalization helper and the moved string escaper, the description is thorough, and the diff is well-scoped. This looks ready for maintainer review. 🚀

Generated by ✅ Contribution Check · 533.5 AIC · ⌖ 22.8 AIC · ⊞ 6K ·

@pelikhan pelikhan marked this pull request as ready for review June 28, 2026 04:48
Copilot AI review requested due to automatic review settings June 28, 2026 04:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This refactor consolidates small duplicated helpers in pkg/cli and centralizes repository “[HOST/]owner/repo” normalization into pkg/repoutil, while also removing a few local helper re-implementations in pkg/workflow and pkg/cli.

Changes:

  • Centralized repo parsing as repoutil.NormalizeRepoForAPI and updated CLI call sites to use it.
  • Extracted shared update-check marker-file helpers into pkg/cli/update_check_state.go and reused them from both update-check paths.
  • Removed a few local helper reinventions (Actions single-quote escaper colocation, permission key sorting via sliceutil.SortedKeys, SHA shortening via stringutil).
Show a summary per file
File Description
pkg/workflow/strings.go Adds shared Actions single-quoted string escaping alongside existing YAML escaping helpers.
pkg/workflow/strings_test.go Adds unit coverage for the Actions single-quote escaping helper.
pkg/workflow/compiler_safe_output_jobs.go Removes the local Actions single-quote escaper in favor of the shared helper.
pkg/workflow/action_cache.go Replaces local SHA-shortening logic with a shared truncation utility.
pkg/repoutil/repoutil.go Introduces NormalizeRepoForAPI in the central repo utilities package.
pkg/repoutil/repoutil_test.go Adds tests for the new repo normalization helper.
pkg/repoutil/README.md Updates repoutil docs to include the new public API helper.
pkg/cli/update_check.go Switches update-check state handling to shared marker-file helpers.
pkg/cli/update_check_state.go New shared helper file for marker path creation, interval gating, and timestamp writes.
pkg/cli/outcome_eval.go Moves host-aware repo normalization usage to repoutil.NormalizeRepoForAPI.
pkg/cli/outcome_eval_test.go Updates existing tests to call the new repoutil normalization helper.
pkg/cli/logs_command.go Updates local-repo detection to use the shared repoutil normalization helper.
pkg/cli/compile_update_check.go Switches compile-update-check state handling to shared marker-file helpers.
pkg/cli/codemod_dependabot_permissions.go Replaces manual key sorting with sliceutil.SortedKeys.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 14/14 changed files
  • Comments generated: 1
  • Review effort level: Low

Comment on lines 671 to 673
func truncateSHAForLog(sha string) string {
if len(sha) > 8 {
return sha[:8]
}
return sha
return stringutil.Truncate(sha, 8)
}
@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, address remaining review feedback, refresh this branch from base, and rerun checks after the update.

Generated by 👨‍🍳 PR Sous Chef · 111.6 AIC · ⌖ 0.953 AIC · ⊞ 17.2K ·

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

PR Triage — Run §28315307719

Category refactor
Risk medium
Priority medium
Score 36/100 — impact 18 · urgency 8 · quality 10
Action batch_review

Deduplicates update-check state marker-file helpers and moves NormalizeRepoForAPI to pkg/repoutil (+131/-123, 14 files). Multiple review comments received; pr-finisher skill requested. 6h old.

Generated by 🔧 PR Triage Agent · 82.5 AIC · ⌖ 10.6 AIC · ⊞ 5.4K ·

@pelikhan pelikhan merged commit 9a6ba96 into main Jun 28, 2026
29 checks passed
@pelikhan pelikhan deleted the copilot/refactor-util-helpers branch June 28, 2026 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants