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

entry: log with spaces between all operands on *ln #25 #26

Merged
merged 1 commit into from
May 4, 2014

Conversation

sirupsen
Copy link
Owner

@sirupsen sirupsen commented May 3, 2014

Fixes #25

If this looks good to you @aarondl, I'll merge it :)

@aarondl
Copy link
Contributor

aarondl commented May 4, 2014

In the sprintlnn function there's an extra Sprintln for no reason. We can simply return msg[:len(msg)-1] otherwise we're impacting performance.

Also this is another issue but I wonder if the savings of loc via code reuse is worth the performance penalty. What I mean is each Xln variant calls it's non-ln variant in an effort to reduce code duplication, but then we get two if statements and two fmt function calls. Just food for thought. This pull request absolutely fixes the issue, now I'm just prematurely optimizing haha.

@sirupsen
Copy link
Owner Author

sirupsen commented May 4, 2014

Oops, good catch. :) Will definitely fix.

I prefer simplicity and consistency over marginal performance gains, unless we can prove it's actually hurting substantially. I've yet to have issues, or hear of, issues with the performance of logrus.

sirupsen added a commit that referenced this pull request May 4, 2014
entry: log with spaces between all operands on *ln #25
@sirupsen sirupsen merged commit 2724908 into master May 4, 2014
@sirupsen sirupsen deleted the printfln-behavior-25 branch May 4, 2014 00:26
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.

Println variants have inconsistent spacing
2 participants