-
Notifications
You must be signed in to change notification settings - Fork 204
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
Remove use of satori uuid package #1306
Conversation
@@ -4,7 +4,6 @@ go 1.13 | |||
|
|||
require ( | |||
github.com/Azure/azure-sdk-for-go v44.0.0+incompatible | |||
github.com/Azure/go-autorest v14.2.0+incompatible // indirect |
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.
these other removals happened due to go mod tidy
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.
LGTM provided tests pass
@matthchr what's the test coverage like? are we safe? |
@Porges the test coverage isn't as good as it could/should be - but it's what we have. I see this package is used by us in azuresqlmanageduser and keyvault. We definitely have tests for KeyVault and from what I can see at least |
I might hold off on this for a bit while Azure SDK for Go updates their references. Then I shouldn’t need to do mod rewriting. |
Tracking this PR in autorest: Azure/autorest.go#520 |
For #1291. Had to fix this with a go.mod rewrite as there are dependencies like the Go Azure SDK that depend on the satori package directly.