-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix stackforms var handling and env update. #323
Conversation
@@ -161,11 +162,13 @@ func NewAPI(opts ...APIOptions) *APIClient { | |||
} | |||
|
|||
func (a *APIClient) Credentials(org *string) runtime.ClientAuthInfoWriter { | |||
var token = a.Config.Token |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get the change from
var token = a.Config.Token
to
var token string
token = a.Config.Token
Would have changed to
token := a.Config.Token
instead and kept the rest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why I would do this: Because Dave Cheney thinks that's good
The key is that
var
acts as a clue that the variable has been deliberately declared as the zero value of the indicated type.When declaring and initialising, consider using the short declaration syntax.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to know.
I will fix that in a future PR
} | ||
} | ||
|
||
return runtime.ClientAuthInfoWriterFunc(func(r runtime.ClientRequest, _ strfmt.Registry) error { | ||
if token == "" { | ||
if len(token) == 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interest: Why did you chose a function call over a simple comparison?
According to this comment by Russ Cox your interested in the is string question, not in the can I access element at index 0 question, so I think the prior version communicates the idea better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I thought that checking by length was safer.
No description provided.