-
Notifications
You must be signed in to change notification settings - Fork 850
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
More complete User-Agent for ASM #557
Conversation
LGTM. On a side-note I know @marstr was looking at updating version reporting to avoid allocations so we should apply that same pattern here at some point. |
OK, waiting for @marstr then. |
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.
This is a good stop-gap fix until we've finished tearing apart the repositories and building up the auto-PR infrastructure.
package management | ||
|
||
var ( | ||
sdkVersion = "8.0.0-beta" |
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.
Ugh, ideally I would have wanted to skip reporting these broken version numbers. However, this is a chicken & egg problem. When I refactor out management into its own repository, I'll have to remember to change the UserAgent string to "Azure-SDK-For-Go-ASM/" so that we can differentiate between this version and the refactored version.
@@ -506,15 +507,23 @@ func vet(service *service) { | |||
} | |||
|
|||
func storageVersion(c *do.Context) { | |||
versionFile := "storage/version.go" | |||
version("storage") |
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 guess it makes sense to have this change for now. However, once everything has been submoduled out, managementVersion and storageVersion should no longer be present.
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.
Should not forget
Fixes #555
PTAL @jhendrixMSFT @marstr