Skip to content
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 caller package name #1102

Merged
merged 1 commit into from Mar 6, 2020
Merged

Conversation

hlcfan
Copy link
Contributor

@hlcfan hlcfan commented Mar 2, 2020

Fix #1089

This is identical to #1099

@thaJeztah
Copy link
Collaborator

Should there be different implementations for Go < 1.14, and Go 1.14 and up?

@hlcfan
Copy link
Contributor Author

hlcfan commented Mar 2, 2020

I think https://go-review.googlesource.com/c/go/+/219118/ should be related.

@hlcfan hlcfan force-pushed the get-right-logrus-pkg-name branch 3 times, most recently from 2548627 to 105b71d Compare March 2, 2020 13:06
@thaJeztah
Copy link
Collaborator

I think https://go-review.googlesource.com/c/go/+/219118/ should be related.

That indicates it should be fixed in the go1.14 final? Is this still an issue in go1.14 (non-rc?)

entry.go Outdated
@@ -419,3 +426,7 @@ func (entry *Entry) sprintlnn(args ...interface{}) string {
msg := fmt.Sprintln(args...)
return msg[:len(msg)-1]
}

func golang1_14() bool {
return strings.HasPrefix(runtime.Version(), goVersion1_14)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this fix is still needed on go1.14 final, it's cleaner to use build tags for this instead of determining go version at runtime; have a file that has a !go1.14 build-tag for older go versions, and an implementation in a file with a go1.14 build-tag for the current version.

See https://github.com/docker/go-connections/pull/68/files for an example

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Updated, please review again.

@hlcfan hlcfan force-pushed the get-right-logrus-pkg-name branch from 105b71d to 7356634 Compare March 2, 2020 13:46
@hlcfan hlcfan force-pushed the get-right-logrus-pkg-name branch from 7356634 to 86a84a9 Compare March 2, 2020 13:47
@thaJeztah
Copy link
Collaborator

Thanks! Last version SGTM (but haven't checked on both go1.13 and go1.14 final)

@hlcfan
Copy link
Contributor Author

hlcfan commented Mar 3, 2020

I think https://go-review.googlesource.com/c/go/+/219118/ should be related.

That indicates it should be fixed in the go1.14 final? Is this still an issue in go1.14 (non-rc?)

I was wrong, that cl wasn't related 😅

@thaJeztah thaJeztah mentioned this pull request Mar 3, 2020
@ergoz
Copy link

ergoz commented Mar 6, 2020

when it will be merged? this bug is critical for me and my colleagues

@dgsb
Copy link
Collaborator

dgsb commented Mar 6, 2020

Thanks for your contributions

@dgsb dgsb merged commit 7ea96a3 into sirupsen:master Mar 6, 2020
@hlcfan hlcfan deleted the get-right-logrus-pkg-name branch March 6, 2020 12:05
@ergoz
Copy link

ergoz commented Mar 9, 2020

thanks for fix!
when new tag will be released?

@Blquinn Blquinn mentioned this pull request Apr 22, 2021
cgxxv pushed a commit to cgxxv/logrus that referenced this pull request Mar 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Report Caller returns logrus files/functions with Go 1.14Beta1
4 participants