-
Notifications
You must be signed in to change notification settings - Fork 89
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
chore: complete migration to mlog and drop logrus #729
Conversation
19bd5e4
to
974e7af
Compare
pkg/cloud/graph.go
Outdated
@@ -49,7 +49,7 @@ func (c *AzureClient) CreateApplication(ctx context.Context, displayName string) | |||
} | |||
appPostOptions.Body.SetDisplayName(to.StringPtr(displayName)) | |||
|
|||
log.Debugf("Creating application with display name=%s", displayName) | |||
mlog.Debug("Creating application", "display name", displayName) |
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.
As we move to structured logging, should the key be camel case? displayName
instead of display name
?
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 am happy either way: which way do you prefer?
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.
Let's use the camel case keys
pkg/cloud/graph.go
Outdated
@@ -120,19 +120,19 @@ func (c *AzureClient) GetApplication(ctx context.Context, displayName string) (* | |||
|
|||
// DeleteServicePrincipal deletes a service principal. | |||
func (c *AzureClient) DeleteServicePrincipal(ctx context.Context, objectID string) error { | |||
log.Debugf("Deleting service principal with object id=%s", objectID) | |||
mlog.Debug("Deleting service principal", "object id", objectID) |
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.
ditto comment for all keys that have 2 words (should we use camel case key name?
)
@@ -103,7 +188,7 @@ func (a *authArgs) Validate() error { | |||
if err != nil || subID.String() == "00000000-0000-0000-0000-000000000000" { | |||
return errors.New("--subscription-id is required (and must be a valid UUID)") | |||
} | |||
log.Infoln("No subscription provided, using selected subscription from Azure CLI:", subID.String()) | |||
mlog.Info("No subscription provided, using selected subscription from Azure CLI", "subscription-id", subID.String()) |
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.
mlog.Info("No subscription provided, using selected subscription from Azure CLI", "subscription-id", subID.String()) | |
mlog.Info("No subscription provided, using selected subscription from Azure CLI", "subscriptionID", subID.String()) |
nit:
Signed-off-by: Monis Khan <[email protected]>
974e7af
to
6d4313a
Compare
@aramase this should be ready now. |
Signed-off-by: Monis Khan [email protected]
Fixes #705
Example logs with
--debug
: