CDTOOL-1328 list version refactor by anthony-gomez-fastly · Pull Request #1774 · fastly/cli

anthony-gomez-fastly · 2026-05-05T19:44:18Z

Change summary

Performance Optimization - Skip ListVersions API call for numeric versions

Changes to pkg/argparser/flags.go:

OptionalServiceVersion.Parse() - Skip API call for numeric versions
- Added early return when user provides numeric version (e.g., --version 1)
- Returns &fastly.Version{Number: n} directly without calling ListVersions API
- Saves 1 API call per command when numeric version is used
- Non-numeric versions (e.g., "latest", "active") still call API as before
OptionalAutoClone.Parse() - Handle unknown version state
- Added stateUnknown check: v.Active == nil && v.Locked == nil
- Updated autoclone condition to include stateUnknown case
- Effect: When --autoclone is provided with numeric version, always clones (defensive behavior) since state is unknown
- Maintains safety while benefiting from optimization

Changes to pkg/argparser/flags_test.go:

Updated test "specific version not in list" - now expects success (version 5) instead of error
Added explicit Active/Locked fields to "version is editable" test

All Submissions:

Have you followed the guidelines in our Contributing document?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

Does your submission pass tests?

Changes to Core Features:

Have you written new tests for your core changes, as applicable?
Have you successfully run tests with your changes locally?

…e specific service version filter API calls Co-Authored-By: Richard Carillo <77027245+rcaril@users.noreply.github.com>

anthony-gomez-fastly · 2026-05-07T18:53:44Z

most of the changes are updating the tests to use the right mock

…tive

rcaril

Minor points - but the PR is good to go as is if you would like to defer these. Thanks for all of your hard work on this @anthony-gomez-fastly !

kpfleming

Yes, I agree this looks great. I thought we had already added --version=staged but I guess we never did!

anthony-gomez-fastly · 2026-05-07T21:32:25Z

@kpfleming added staging!

anthony-gomez-fastly · 2026-05-05T19:44:18Z

Change summary

Performance Optimization - Skip ListVersions API call for numeric versions

Changes to pkg/argparser/flags.go:

OptionalServiceVersion.Parse() - Skip API call for numeric versions
- Added early return when user provides numeric version (e.g., --version 1)
- Returns &fastly.Version{Number: n} directly without calling ListVersions API
- Saves 1 API call per command when numeric version is used
- Non-numeric versions (e.g., "latest", "active") still call API as before
OptionalAutoClone.Parse() - Handle unknown version state
- Added stateUnknown check: v.Active == nil && v.Locked == nil
- Updated autoclone condition to include stateUnknown case
- Effect: When --autoclone is provided with numeric version, always clones (defensive behavior) since state is unknown
- Maintains safety while benefiting from optimization

Changes to pkg/argparser/flags_test.go:

Updated test "specific version not in list" - now expects success (version 5) instead of error
Added explicit Active/Locked fields to "version is editable" test

All Submissions:

Have you followed the guidelines in our Contributing document?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

Does your submission pass tests?

Changes to Core Features:

Have you written new tests for your core changes, as applicable?
Have you successfully run tests with your changes locally?

…e specific service version filter API calls Co-Authored-By: Richard Carillo <77027245+rcaril@users.noreply.github.com>

anthony-gomez-fastly · 2026-05-07T18:53:44Z

most of the changes are updating the tests to use the right mock

…tive

rcaril

Minor points - but the PR is good to go as is if you would like to defer these. Thanks for all of your hard work on this @anthony-gomez-fastly !

kpfleming

Yes, I agree this looks great. I thought we had already added --version=staged but I guess we never did!

anthony-gomez-fastly · 2026-05-07T21:32:25Z

@kpfleming added staging!

anthony-gomez-fastly · 2026-05-05T19:44:18Z

Change summary

Performance Optimization - Skip ListVersions API call for numeric versions

Changes to pkg/argparser/flags.go:

OptionalServiceVersion.Parse() - Skip API call for numeric versions
- Added early return when user provides numeric version (e.g., --version 1)
- Returns &fastly.Version{Number: n} directly without calling ListVersions API
- Saves 1 API call per command when numeric version is used
- Non-numeric versions (e.g., "latest", "active") still call API as before
OptionalAutoClone.Parse() - Handle unknown version state
- Added stateUnknown check: v.Active == nil && v.Locked == nil
- Updated autoclone condition to include stateUnknown case
- Effect: When --autoclone is provided with numeric version, always clones (defensive behavior) since state is unknown
- Maintains safety while benefiting from optimization

Changes to pkg/argparser/flags_test.go:

Updated test "specific version not in list" - now expects success (version 5) instead of error
Added explicit Active/Locked fields to "version is editable" test

All Submissions:

Have you followed the guidelines in our Contributing document?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

Does your submission pass tests?

Changes to Core Features:

Have you written new tests for your core changes, as applicable?
Have you successfully run tests with your changes locally?

…e specific service version filter API calls Co-Authored-By: Richard Carillo <77027245+rcaril@users.noreply.github.com>

anthony-gomez-fastly · 2026-05-07T18:53:44Z

most of the changes are updating the tests to use the right mock

…tive

rcaril

Minor points - but the PR is good to go as is if you would like to defer these. Thanks for all of your hard work on this @anthony-gomez-fastly !

kpfleming

Yes, I agree this looks great. I thought we had already added --version=staged but I guess we never did!

anthony-gomez-fastly · 2026-05-07T21:32:25Z

@kpfleming added staging!

anthony-gomez-fastly · 2026-05-05T19:44:18Z

Change summary

Performance Optimization - Skip ListVersions API call for numeric versions

Changes to pkg/argparser/flags.go:

OptionalServiceVersion.Parse() - Skip API call for numeric versions
- Added early return when user provides numeric version (e.g., --version 1)
- Returns &fastly.Version{Number: n} directly without calling ListVersions API
- Saves 1 API call per command when numeric version is used
- Non-numeric versions (e.g., "latest", "active") still call API as before
OptionalAutoClone.Parse() - Handle unknown version state
- Added stateUnknown check: v.Active == nil && v.Locked == nil
- Updated autoclone condition to include stateUnknown case
- Effect: When --autoclone is provided with numeric version, always clones (defensive behavior) since state is unknown
- Maintains safety while benefiting from optimization

Changes to pkg/argparser/flags_test.go:

Updated test "specific version not in list" - now expects success (version 5) instead of error
Added explicit Active/Locked fields to "version is editable" test

All Submissions:

Have you followed the guidelines in our Contributing document?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

Does your submission pass tests?

Changes to Core Features:

Have you written new tests for your core changes, as applicable?
Have you successfully run tests with your changes locally?

…e specific service version filter API calls Co-Authored-By: Richard Carillo <77027245+rcaril@users.noreply.github.com>

anthony-gomez-fastly · 2026-05-07T18:53:44Z

most of the changes are updating the tests to use the right mock

…tive

rcaril

Minor points - but the PR is good to go as is if you would like to defer these. Thanks for all of your hard work on this @anthony-gomez-fastly !

kpfleming

Yes, I agree this looks great. I thought we had already added --version=staged but I guess we never did!

anthony-gomez-fastly · 2026-05-07T21:32:25Z

@kpfleming added staging!

anthony-gomez-fastly · 2026-05-05T19:44:18Z

Change summary

Performance Optimization - Skip ListVersions API call for numeric versions

Changes to pkg/argparser/flags.go:

OptionalServiceVersion.Parse() - Skip API call for numeric versions
- Added early return when user provides numeric version (e.g., --version 1)
- Returns &fastly.Version{Number: n} directly without calling ListVersions API
- Saves 1 API call per command when numeric version is used
- Non-numeric versions (e.g., "latest", "active") still call API as before
OptionalAutoClone.Parse() - Handle unknown version state
- Added stateUnknown check: v.Active == nil && v.Locked == nil
- Updated autoclone condition to include stateUnknown case
- Effect: When --autoclone is provided with numeric version, always clones (defensive behavior) since state is unknown
- Maintains safety while benefiting from optimization

Changes to pkg/argparser/flags_test.go:

Updated test "specific version not in list" - now expects success (version 5) instead of error
Added explicit Active/Locked fields to "version is editable" test

All Submissions:

Have you followed the guidelines in our Contributing document?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

Does your submission pass tests?

Changes to Core Features:

Have you written new tests for your core changes, as applicable?
Have you successfully run tests with your changes locally?

…e specific service version filter API calls Co-Authored-By: Richard Carillo <77027245+rcaril@users.noreply.github.com>

anthony-gomez-fastly · 2026-05-07T18:53:44Z

most of the changes are updating the tests to use the right mock

…tive

rcaril

Minor points - but the PR is good to go as is if you would like to defer these. Thanks for all of your hard work on this @anthony-gomez-fastly !

kpfleming

Yes, I agree this looks great. I thought we had already added --version=staged but I guess we never did!

anthony-gomez-fastly · 2026-05-07T21:32:25Z

@kpfleming added staging!

anthony-gomez-fastly · 2026-05-05T19:44:18Z

Change summary

Performance Optimization - Skip ListVersions API call for numeric versions

Changes to pkg/argparser/flags.go:

OptionalServiceVersion.Parse() - Skip API call for numeric versions
- Added early return when user provides numeric version (e.g., --version 1)
- Returns &fastly.Version{Number: n} directly without calling ListVersions API
- Saves 1 API call per command when numeric version is used
- Non-numeric versions (e.g., "latest", "active") still call API as before
OptionalAutoClone.Parse() - Handle unknown version state
- Added stateUnknown check: v.Active == nil && v.Locked == nil
- Updated autoclone condition to include stateUnknown case
- Effect: When --autoclone is provided with numeric version, always clones (defensive behavior) since state is unknown
- Maintains safety while benefiting from optimization

Changes to pkg/argparser/flags_test.go:

Updated test "specific version not in list" - now expects success (version 5) instead of error
Added explicit Active/Locked fields to "version is editable" test

All Submissions:

Have you followed the guidelines in our Contributing document?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

Does your submission pass tests?

Changes to Core Features:

Have you written new tests for your core changes, as applicable?
Have you successfully run tests with your changes locally?

…e specific service version filter API calls Co-Authored-By: Richard Carillo <77027245+rcaril@users.noreply.github.com>

anthony-gomez-fastly · 2026-05-07T18:53:44Z

most of the changes are updating the tests to use the right mock

…tive

rcaril

Minor points - but the PR is good to go as is if you would like to defer these. Thanks for all of your hard work on this @anthony-gomez-fastly !

kpfleming

Yes, I agree this looks great. I thought we had already added --version=staged but I guess we never did!

anthony-gomez-fastly · 2026-05-07T21:32:25Z

@kpfleming added staging!

anthony-gomez-fastly · 2026-05-05T19:44:18Z

Change summary

Performance Optimization - Skip ListVersions API call for numeric versions

Changes to pkg/argparser/flags.go:

OptionalServiceVersion.Parse() - Skip API call for numeric versions
- Added early return when user provides numeric version (e.g., --version 1)
- Returns &fastly.Version{Number: n} directly without calling ListVersions API
- Saves 1 API call per command when numeric version is used
- Non-numeric versions (e.g., "latest", "active") still call API as before
OptionalAutoClone.Parse() - Handle unknown version state
- Added stateUnknown check: v.Active == nil && v.Locked == nil
- Updated autoclone condition to include stateUnknown case
- Effect: When --autoclone is provided with numeric version, always clones (defensive behavior) since state is unknown
- Maintains safety while benefiting from optimization

Changes to pkg/argparser/flags_test.go:

Updated test "specific version not in list" - now expects success (version 5) instead of error
Added explicit Active/Locked fields to "version is editable" test

All Submissions:

Have you followed the guidelines in our Contributing document?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

Does your submission pass tests?

Changes to Core Features:

Have you written new tests for your core changes, as applicable?
Have you successfully run tests with your changes locally?

…e specific service version filter API calls Co-Authored-By: Richard Carillo <77027245+rcaril@users.noreply.github.com>

anthony-gomez-fastly · 2026-05-07T18:53:44Z

most of the changes are updating the tests to use the right mock

…tive

rcaril

Minor points - but the PR is good to go as is if you would like to defer these. Thanks for all of your hard work on this @anthony-gomez-fastly !

kpfleming

Yes, I agree this looks great. I thought we had already added --version=staged but I guess we never did!

anthony-gomez-fastly · 2026-05-07T21:32:25Z

@kpfleming added staging!

anthony-gomez-fastly · 2026-05-05T19:44:18Z

Change summary

Performance Optimization - Skip ListVersions API call for numeric versions

Changes to pkg/argparser/flags.go:

OptionalServiceVersion.Parse() - Skip API call for numeric versions
- Added early return when user provides numeric version (e.g., --version 1)
- Returns &fastly.Version{Number: n} directly without calling ListVersions API
- Saves 1 API call per command when numeric version is used
- Non-numeric versions (e.g., "latest", "active") still call API as before
OptionalAutoClone.Parse() - Handle unknown version state
- Added stateUnknown check: v.Active == nil && v.Locked == nil
- Updated autoclone condition to include stateUnknown case
- Effect: When --autoclone is provided with numeric version, always clones (defensive behavior) since state is unknown
- Maintains safety while benefiting from optimization

Changes to pkg/argparser/flags_test.go:

Updated test "specific version not in list" - now expects success (version 5) instead of error
Added explicit Active/Locked fields to "version is editable" test

All Submissions:

Have you followed the guidelines in our Contributing document?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

Does your submission pass tests?

Changes to Core Features:

Have you written new tests for your core changes, as applicable?
Have you successfully run tests with your changes locally?

…e specific service version filter API calls Co-Authored-By: Richard Carillo <77027245+rcaril@users.noreply.github.com>

anthony-gomez-fastly · 2026-05-07T18:53:44Z

most of the changes are updating the tests to use the right mock

…tive

rcaril

Minor points - but the PR is good to go as is if you would like to defer these. Thanks for all of your hard work on this @anthony-gomez-fastly !

kpfleming

Yes, I agree this looks great. I thought we had already added --version=staged but I guess we never did!

anthony-gomez-fastly · 2026-05-07T21:32:25Z

@kpfleming added staging!

anthony-gomez-fastly force-pushed the rcaril/CDTOOL-1328-list-version-refactor branch 5 times, most recently from a740a16 to 429df46Compare May 7, 2026 18:29

modified flags.go parse function to no longer use list and instead us…
0c76483
…e specific service version filter API calls Co-Authored-By: Richard Carillo <77027245+rcaril@users.noreply.github.com>

anthony-gomez-fastly force-pushed the rcaril/CDTOOL-1328-list-version-refactor branch from 429df46 to 0c76483Compare May 7, 2026 18:40

anthony-gomez-fastly marked this pull request as ready for review May 7, 2026 18:51

anthony-gomez-fastly requested a review from a team as a code owner May 7, 2026 18:51

anthony-gomez-fastly requested review from kpfleming and rcaril May 7, 2026 18:51

anthony-gomez-fastly commented May 7, 2026
View reviewed changes

Comment threadpkg/argparser/flags.go

anthony-gomez-fastly commented May 7, 2026
View reviewed changes

Comment threadpkg/argparser/flags.go

change unset version to first try active, then return latest if no ac…
eb112fa
…tive

rcaril approved these changes May 7, 2026
View reviewed changes

Comment threadpkg/argparser/flags.go
Comment threadpkg/commands/service/resourcelink/resourcelink_test.go Outdated

kpfleming approved these changes May 7, 2026
View reviewed changes

anthony-gomez-fastly added 3 commits May 7, 2026 17:25

remove extra calls
97fad1f

add staged version filter
63d3a0c

added documentation for the same
224206d

fix lint
a545634

anthony-gomez-fastly merged commit 576f624 into mainMay 8, 2026
13 checks passed

anthony-gomez-fastly deleted the rcaril/CDTOOL-1328-list-version-refactor branch May 8, 2026 13:23

BrewTestBot mentioned this pull request May 8, 2026
fastly 15.0.0 Homebrew/homebrew-core#281607
Merged