Skip to content

Commit cb30df8

Browse files
authored
refactor: remove legacy CLIVersion field and SetVersion() function (#429)
refactor: remove vestigial CLIVersion field and SetVersion option The CLIVersion field and SetVersion functional option on ClientFactory were a workaround for a circular dependency that no longer exists. The API client reads version from context via slackcontext.Version(ctx), making this indirection unnecessary. Replace with direct version.Raw() calls.
1 parent 4c7219f commit cb30df8

3 files changed

Lines changed: 5 additions & 18 deletions

File tree

cmd/root.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ func Init(ctx context.Context) (*cobra.Command, *shared.ClientFactory) {
148148
// updateNotification will check for an update in the background and print a message after the command runs
149149
var updateNotification *update.UpdateNotification
150150

151-
clients = shared.NewClientFactory(shared.SetVersion(version.Raw()))
151+
clients = shared.NewClientFactory()
152152
rootCmd := NewRootCommand(clients, updateNotification)
153153

154154
// Support `--version` by setting root command's `Version` and custom template.
@@ -265,7 +265,7 @@ func InitConfig(ctx context.Context, clients *shared.ClientFactory, rootCmd *cob
265265

266266
// Init clients that use flags
267267
clients.Config.APIHostResolved = clients.Auth().ResolveAPIHost(ctx, clients.Config.APIHostFlag, nil)
268-
clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved, clients.CLIVersion)
268+
clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved, version.Raw())
269269

270270
// Init System ID
271271
if systemID, err := clients.Config.SystemConfig.InitSystemID(ctx); err != nil {
@@ -297,7 +297,7 @@ func InitConfig(ctx context.Context, clients *shared.ClientFactory, rootCmd *cob
297297
clients.Config.LoadExperiments(ctx, clients.IO.PrintDebug)
298298
style.ToggleLipgloss(clients.Config.WithExperimentOn(experiment.Lipgloss))
299299
// TODO(slackcontext) Consolidate storing CLI version to slackcontext
300-
clients.Config.Version = clients.CLIVersion
300+
clients.Config.Version = version.Raw()
301301

302302
// The domain auths (token->domain) shouldn't change for the execution of the CLI so preload them into config!
303303
clients.Config.DomainAuthTokens = clients.Auth().MapAuthTokensToDomains(ctx)

internal/shared/clients.go

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ type ClientFactory struct {
4545
API func() api.APIInterface
4646
AppClient func() *app.Client
4747
Auth func() auth.AuthInterface
48-
CLIVersion string
4948
Config *config.Config
5049
EventTracker tracking.TrackingManager
5150
HookExecutor hooks.HookExecutor
@@ -92,13 +91,6 @@ func NewClientFactory(options ...func(*ClientFactory)) *ClientFactory {
9291
clients.Auth = clients.defaultAuthFunc
9392
clients.Browser = clients.defaultBrowserFunc
9493

95-
// TODO: Temporary hack to get around circular dependency in internal/api/client.go since that imports version
96-
// Follows pattern demonstrated by the GitHub CLI here https://github.com/cli/cli/blob/5a46c1cab601a3394caa8de85adb14f909b811e9/pkg/cmd/factory/default.go#L29
97-
// Used by the APIClient for its userAgent
98-
// Currently needed because trying to get the version of the CLI from internal/version would cause a circular dependency
99-
// userAgent can get Slack CLI version from context which is defined in main.go, this approach bypass circular dependency. The clients.CLIVersion is retained for future code refactor purpose and serve SetVersion function
100-
clients.CLIVersion = ""
101-
10294
// Custom values set by functional options
10395
// Learn more: https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis
10496
for _, option := range options {
@@ -322,12 +314,6 @@ func DebugMode(c *ClientFactory) {
322314
c.Config.DebugEnabled = true
323315
}
324316

325-
// SetVersion is a functional option that sets the Cli version that the API Client references
326-
// Learn more: https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis
327-
func SetVersion(version string) func(c *ClientFactory) {
328-
return func(c *ClientFactory) { c.CLIVersion = version }
329-
}
330-
331317
// getDevHostname returns the hostname of the given URL if it is dev or a numbered dev instance
332318
func getDevHostname(host string) string {
333319
if host == "" {

main.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ func main() {
8080
func recoveryFunc() {
8181
// in the event of a panic, log panic
8282
if r := recover(); r != nil {
83-
var clients = shared.NewClientFactory(shared.SetVersion(version.Raw()))
83+
var clients = shared.NewClientFactory()
84+
clients.Config.Version = version.Raw()
8485

8586
var ctx = context.Background()
8687
ctx = slackcontext.SetSessionID(ctx, uuid.New().String())

0 commit comments

Comments
 (0)