Skip to content

Conversation

luxass
Copy link
Contributor

@luxass luxass commented Aug 26, 2025

resolves #11425

@luxass luxass temporarily deployed to cli-automation August 26, 2025 18:17 — with GitHub Actions Inactive
@luxass luxass force-pushed the cache-delete-with-ref branch 2 times, most recently from daf902a to 0d1629a Compare August 26, 2025 18:28
@luxass luxass marked this pull request as ready for review August 26, 2025 18:29
@Copilot Copilot AI review requested due to automatic review settings August 26, 2025 18:29
@luxass luxass requested a review from a team as a code owner August 26, 2025 18:29
@luxass luxass requested a review from andyfeller August 26, 2025 18:29
@luxass luxass temporarily deployed to cli-automation August 26, 2025 18:29 — with GitHub Actions Inactive
@cliAutomation cliAutomation added the external pull request originating outside of the CLI core team label Aug 26, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for a --ref flag to the gh cache delete command, allowing users to narrow down cache deletion to a specific Git reference (branch or PR). The enhancement enables more precise cache management by specifying refs like refs/heads/branch-name or refs/pull/123/merge.

  • Added --ref flag to cache delete command with validation logic
  • Updated API integration to include ref parameter in delete requests
  • Added comprehensive test coverage for the new functionality

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
pkg/cmd/cache/delete/delete.go Implements --ref flag with validation and API parameter handling
pkg/cmd/cache/delete/delete_test.go Adds test cases for --ref flag validation and API integration

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Member

@andyfeller andyfeller left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution, @luxass! 🫶

This looks pretty good with some minor suggestions to ensure users have clear guidance.

luxass added 4 commits August 27, 2025 04:56
- Introduced `--ref` flag to narrow down cache deletion to a specific branch or PR reference.
- Updated command examples to include usage of `--ref`.
- Added validation to ensure `--ref` cannot be used with `--all` and must accompany a cache key/ID.
* Implemented test cases for handling the `--ref` flag in cache deletion commands.
* Added validation for using `--ref` with cache key and ID.
* Ensured proper error messages are returned for invalid usage of `--ref`.
* Clarified the description of the `--ref` flag to specify its purpose more accurately.
* Ensures users understand that it narrows down deletion by cache key and ref.
* Changed the error message from "--ref cannot be used without cache key/ID" to "must provide a cache key" for clarity.
* Updated corresponding test case to reflect the new error message.
@luxass luxass force-pushed the cache-delete-with-ref branch from f2102ad to 047326f Compare August 27, 2025 02:57
Copy link
Member

@babakks babakks left a comment

Choose a reason for hiding this comment

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

The PR looks very good, @luxass! 🎉

To finalise the changes, I think we need to cover scenarios below. When you're done I'll go through the testing part.

1. Use --ref with a cache ID

The --ref option only applies to cases where the argument is a cache key. Therefore we need to add another validation check to make sure --ref is not used with a cache ID, with an error message like this:

--ref cannot be used with a cache ID

For this we can extract a small function like below and use it in both cmd.RunE (for validation) and deleteCaches.

func parseCacheID(arg string) (int, bool) {
    id, err := strconv.Atoi(arg)
    return id, err == nil
}

2. Better error message when --ref not found

Currently, if the user runs cache delete <cache-key> --ref <ref> where ref is invalid (or doesn't exist) then they'll see an error like this:

X Could not find a cache matching <cache-key> in <repo>

Since the user has provided the --ref option I think we should hint at the ref value in the error message. Something like this makes sense to me:

X Could not find a cache matching <cache-key> (with ref <ref>) in <repo>

Note that this format should only be used when there's a non-empty --ref value.

luxass added 3 commits August 27, 2025 19:37
* Replaced direct conversion of cache ID string to int with a dedicated `parseCacheID` function.
* Improved readability and maintainability of the `deleteCaches` function.
* Added validation to ensure that the `--ref` flag cannot be used when a cache ID is provided.
* Updated error message to include the reference when a cache is not found with the specified `--ref`.
* Ensure that the `--ref` flag cannot be used in conjunction with a cache ID.
* Update error handling to provide clear feedback when this condition is violated.
* Add tests to cover scenarios involving the `--ref` flag and cache ID.
@luxass luxass requested a review from babakks August 27, 2025 18:02
Copy link
Member

@babakks babakks left a comment

Choose a reason for hiding this comment

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

Thanks for the changes, @luxass! 🙏

I went through the manual testing and noticed a few things. I'll now push some commits to fix the comments I just made below. With those, I think the PR is ready to be merged, but I'll ask for a another review from @andyfeller since I have made some changes.

@babakks babakks requested a review from andyfeller August 28, 2025 10:20
@babakks
Copy link
Member

babakks commented Aug 28, 2025

@andyfeller I just resolved the comments from my last review since they're all sorted in the commits I pushed. I think the PR is good to be merged, but will appreciate your eyes on it. 🙏

@mxie mxie dismissed andyfeller’s stale review September 3, 2025 15:04

dismissing to move this forward

@mxie mxie merged commit 926e293 into cli:trunk Sep 3, 2025
12 of 14 checks passed
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Sep 12, 2025
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [cli/cli](https://github.com/cli/cli) | minor | `v2.78.0` -> `v2.79.0` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>cli/cli (cli/cli)</summary>

### [`v2.79.0`](https://github.com/cli/cli/releases/tag/v2.79.0): GitHub CLI 2.79.0

[Compare Source](cli/cli@v2.78.0...v2.79.0)

#### Advanced Issue Search Support

The GitHub CLI now supports advanced issue search syntax using:

- Searching issues: `gh search issues <advanced issue search query>`
- Searching pull requests: `gh search prs <advanced issue search query>`
- While listing issues: `gh issue list --search <advanced issue search query>`
- While listing pull requests: `gh pr list --search <advanced issue search query>`

For more information about advanced issue search syntax, see: "[Filtering and Searching Issues and Merge Requests](https://docs.github.com/en/issues/tracking-your-work-with-issues/using-issues/filtering-and-searching-issues-and-pull-requests#building-advanced-filters-for-issues)"

#### Copy OAuth Code Automatically

The GitHub CLI now supports writing the OAuth one-time pass code to the clipboard automatically during authentication:

- While logging in: `gh auth login --clipboard` / `gh auth login -c`
- While refreshing the token: `gh auth refresh --clipboard` / `gh auth refresh -c`

#### What's Changed

##### ✨ Features

- feat: `gh auth` Automatically copy one-time OAuth code to clipboard by [@&#8203;ankddev](https://github.com/ankddev) in [#&#8203;11518](cli/cli#11518)
- feat: add support for `--ref` in `gh cache delete` by [@&#8203;luxass](https://github.com/luxass) in [#&#8203;11592](cli/cli#11592)
- Use advanced issue search by [@&#8203;babakks](https://github.com/babakks) in [#&#8203;11638](cli/cli#11638)

##### 📚 Docs & Chores

- docs(release create): difference `--generate-notes` and `--notes-from-tag` by [@&#8203;ankddev](https://github.com/ankddev) in [#&#8203;11534](cli/cli#11534)
- refactor tests: use `slices.Equal` to simplify code by [@&#8203;minxinyi](https://github.com/minxinyi) in [#&#8203;11364](cli/cli#11364)
- Remove mention of public preview in trustedroot.go by [@&#8203;jkylekelly](https://github.com/jkylekelly) in [#&#8203;11652](cli/cli#11652)

##### :dependabot: Dependencies

- Bump sigstore/rekor to v1.4.1 by [@&#8203;BagToad](https://github.com/BagToad) in [#&#8203;11654](cli/cli#11654)
- chore(deps): bump actions/stale from 9 to 10 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;11663](cli/cli#11663)
- chore(deps): bump actions/setup-go from 5 to 6 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;11662](cli/cli#11662)

#### New Contributors

- [@&#8203;minxinyi](https://github.com/minxinyi) made their first contribution in [#&#8203;11364](cli/cli#11364)
- [@&#8203;jkylekelly](https://github.com/jkylekelly) made their first contribution in [#&#8203;11652](cli/cli#11652)
- [@&#8203;luxass](https://github.com/luxass) made their first contribution in [#&#8203;11592](cli/cli#11592)

**Full Changelog**: <cli/cli@v2.78.0...v2.79.0>

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MS45OC4xIiwidXBkYXRlZEluVmVyIjoiNDEuOTguMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90Il19-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external pull request originating outside of the CLI core team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support --ref parameter in gh cache delete
5 participants