From e114959c41ccaa83d1749eac5ab1bad73583bfe5 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Thu, 26 Mar 2026 16:48:59 -0700 Subject: [PATCH] refactor: remove version.Get() and use only version.Raw() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the v-prefix enforcement from Get() into init(), ensuring Version is normalized at startup. This eliminates the confusing Get() vs Raw() distinction — Raw() is now the single way to access the version. --- cmd/doctor/doctor_test.go | 2 +- cmd/root.go | 6 +++--- cmd/upgrade/upgrade.go | 2 +- cmd/version/version.go | 2 +- internal/pkg/auth/login.go | 6 +++--- internal/version/version.go | 14 +++++++------- internal/version/version_test.go | 8 ++------ 7 files changed, 18 insertions(+), 22 deletions(-) diff --git a/cmd/doctor/doctor_test.go b/cmd/doctor/doctor_test.go index fdefbb41..25ef5796 100644 --- a/cmd/doctor/doctor_test.go +++ b/cmd/doctor/doctor_test.go @@ -38,7 +38,7 @@ import ( ) func TestDoctorCommand(t *testing.T) { - expectedCLIVersion := version.Get() + expectedCLIVersion := version.Raw() expectedCredentials := types.SlackAuth{ TeamDomain: "team123", TeamID: "T123", diff --git a/cmd/root.go b/cmd/root.go index 4d472450..e9282c69 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -125,14 +125,14 @@ func NewRootCommand(clients *shared.ClientFactory, updateNotification *update.Up }) // Check for an CLI update in the background while the command runs - updateNotification = update.New(clients, version.Get(), "SLACK_SKIP_UPDATE") + updateNotification = update.New(clients, version.Raw(), "SLACK_SKIP_UPDATE") updateNotification.CheckForUpdateInBackground(ctx, false) return nil }, PersistentPostRunE: func(cmd *cobra.Command, args []string) error { // TODO: since commands are moving to `*E` cobra lifecycle methods, this method may not be invoked if those earlier lifecycle methods return an error. Maybe move this to the cleanup() method below? but maybe this is OK, no need to prompt users to update if they encounter an error? // when the command is `slack update` - return updateNotification.PrintAndPromptUpdates(cmd, version.Get()) + return updateNotification.PrintAndPromptUpdates(cmd, version.Raw()) }, RunE: func(cmd *cobra.Command, args []string) error { return cmd.Help() @@ -153,7 +153,7 @@ func Init(ctx context.Context) (*cobra.Command, *shared.ClientFactory) { // Support `--version` by setting root command's `Version` and custom template. // Add a newline to `SetVersionTemplate` to display correctly on terminals. - rootCmd.Version = version.Get() + rootCmd.Version = version.Raw() rootCmd.SetVersionTemplate(versioncmd.Template() + "\n") // Add subcommands (each subcommand may add their own child subcommands) diff --git a/cmd/upgrade/upgrade.go b/cmd/upgrade/upgrade.go index edb51bd1..325fdad9 100644 --- a/cmd/upgrade/upgrade.go +++ b/cmd/upgrade/upgrade.go @@ -55,7 +55,7 @@ func NewCommand(clients *shared.ClientFactory) *cobra.Command { // When there are updates, the function will *not* print a message because the root command handles printing update notifications. func checkForUpdates(clients *shared.ClientFactory, cmd *cobra.Command) error { ctx := cmd.Context() - updateNotification := update.New(clients, version.Get(), "SLACK_SKIP_UPDATE") + updateNotification := update.New(clients, version.Raw(), "SLACK_SKIP_UPDATE") // TODO(@mbrooks) This update check is happening at the same time as the root command's `CheckForUpdateInBackground`. // The difference between the two is that this update check is forced while the root command runs every 24 hours. diff --git a/cmd/version/version.go b/cmd/version/version.go index 792fc547..03d00c7d 100644 --- a/cmd/version/version.go +++ b/cmd/version/version.go @@ -81,7 +81,7 @@ func NewCommand(clients *shared.ClientFactory) *cobra.Command { func Template() string { processName := cmdutil.GetProcessName() - version := version.Get() + version := version.Raw() return fmt.Sprintf(style.Secondary("Using %s %s"), processName, version) } diff --git a/internal/pkg/auth/login.go b/internal/pkg/auth/login.go index e572c43a..3194755f 100644 --- a/internal/pkg/auth/login.go +++ b/internal/pkg/auth/login.go @@ -147,7 +147,7 @@ func createNewAuth(ctx context.Context, apiClient api.APIInterface, authClient a return types.SlackAuth{}, "", err } - authExchangeRes, err := apiClient.ExchangeAuthTicket(ctx, authTicket, challengeCode, version.Get()) + authExchangeRes, err := apiClient.ExchangeAuthTicket(ctx, authTicket, challengeCode, version.Raw()) if err != nil { return types.SlackAuth{}, "", err } @@ -162,7 +162,7 @@ func requestAuthTicket(ctx context.Context, apiClient api.APIInterface, io iostr span, ctx = opentracing.StartSpanFromContext(ctx, "requestAuthTicket") defer span.Finish() - cliVersion := semver.MajorMinor(version.Get()) + cliVersion := semver.MajorMinor(version.Raw()) // Get a ticket from Slack if authTicketResult, err := apiClient.GenerateAuthTicket(ctx, cliVersion, noRotation); err != nil { @@ -246,7 +246,7 @@ func LoginNoPrompt(ctx context.Context, clients *shared.ClientFactory, ticketArg // existing ticket request, try to exchange if ticketArg != "" && challengeCodeArg != "" { - authExchangeRes, err := clients.API().ExchangeAuthTicket(ctx, ticketArg, challengeCodeArg, version.Get()) + authExchangeRes, err := clients.API().ExchangeAuthTicket(ctx, ticketArg, challengeCodeArg, version.Raw()) if err != nil || !authExchangeRes.IsReady { return types.SlackAuth{}, "", err } diff --git a/internal/version/version.go b/internal/version/version.go index 4ea807c9..5fcf38dc 100644 --- a/internal/version/version.go +++ b/internal/version/version.go @@ -33,6 +33,7 @@ func init() { if envVersion := getVersionFromEnv(); envVersion != "" { Version = envVersion } + Version = ensurePrefix(Version) } // getVersionFromEnv will return the formatted version from EnvTestVersion otherwise "". @@ -40,16 +41,15 @@ func getVersionFromEnv() string { return strings.Trim(os.Getenv(EnvTestVersion), " ") } -// Get the version and format it (e.g. `v1.0.0`) -func Get() string { - version := Version - if match, _ := regexp.MatchString(`^[^v]`, version); match { - version = "v" + version +// ensurePrefix ensures that the version string has a "v" prefix. +func ensurePrefix(v string) string { + if match, _ := regexp.MatchString(`^[^v]`, v); match { + return "v" + v } - return version + return v } -// Raw returns the raw, unformatted version +// Raw returns the version func Raw() string { return Version } diff --git a/internal/version/version_test.go b/internal/version/version_test.go index 535504bf..15953a68 100644 --- a/internal/version/version_test.go +++ b/internal/version/version_test.go @@ -27,8 +27,7 @@ func TestVersion(t *testing.T) { assert.True(t, len(v) > 0, "some default value exists") } -// Test overriding the Version with an environment variable -func Test_Get(t *testing.T) { +func Test_ensurePrefix(t *testing.T) { tests := map[string]struct { version string expected string @@ -52,10 +51,7 @@ func Test_Get(t *testing.T) { } for name, tc := range tests { t.Run(name, func(t *testing.T) { - original := Version - defer func() { Version = original }() - Version = tc.version - assert.Equal(t, tc.expected, Get()) + assert.Equal(t, tc.expected, ensurePrefix(tc.version)) }) } }