-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Update from Sirupsen -> sirupsen #1573
Conversation
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.
LGMT, but I think that we shouldn't leave code in comments, but track the fix in separate issue/retrieve the old version to fix from git.
common/logging.go
Outdated
@@ -242,24 +241,24 @@ func setupFluentD(logOpts map[string]string, debug bool) { | |||
// setupLogstash sets up and configures Logstash with the provided options in | |||
// logOpts. If some options are not provided, sensible defaults are used. | |||
/// TODO fix me later - needs to be tested with a working logstash setup. |
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.
Can we remove it and grab it from git later/track it in a separate issue?
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.
What I've started doing is to file a GH issues and refer to it from the comment in the code ie in the form:
// GH-1231: text
Also update all other dependencies that were using Sirupsen instead of sirupsen Signed-off-by: André Martins <andre@cilium.io>
Signed-off-by: André Martins <andre@cilium.io>
ec10001
to
7053218
Compare
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!
@@ -25,6 +25,10 @@ Documentation | |||
------------- | |||
* Add here | |||
|
|||
Other | |||
----- | |||
* Update Sirupsen/logrus to sirupsen/logrus (1573_) |
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.
👍 Thank you!
No description provided.