Skip to content

Update logging statements for zig std.log breaking change - #147

Closed
heidezomp wants to merge 1 commit into
zigtools:masterfrom
heidezomp:update-for-std-log-breaking-change
Closed

Update logging statements for zig std.log breaking change#147
heidezomp wants to merge 1 commit into
zigtools:masterfrom
heidezomp:update-for-std-log-breaking-change

Conversation

@heidezomp

Copy link
Copy Markdown

I implemented ziglang/zig#6046 with breaking changes to std.log and then noticed that my zls wouldn't build anymore... so I made it build again 😄

Marked this PR as draft since the upstream zig PR isn't merged yet.

Comment threadsrc/main.zig
@@ -1,4 +1,5 @@
const std = @import("std");
const main_log = std.log.scoped(.main);

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I don't really like that I had to call this one main_log instead of just log like in the other files, since otherwise it would clash with the custom log implementation. I argued for renaming the log implementation to "logger" to prevent this name collision here ziglang/zig#6039 (comment):

The intended usage pattern of const log = std.log.scoped(.main); log.err("oops", .{}); can be used everywhere except in the root source file, if that file overrides the log implementation (since it would result in a naming conflict). Is this a problem? Maybe the log implementation could be renamed to "logger" instead?

Do you have any ideas to improve this, or do you think it's fine the way it is?

@alexnask

Copy link
Copy Markdown
Member

Hi, thanks for the contributioin.
I just updated this before checking for new issues and PRs :P
I went with logger for the main scoped log for now :)

@heidezomp
heidezomp deleted the update-for-std-log-breaking-change branch August 14, 2020 11:39
@heidezomp

Copy link
Copy Markdown
Author

That's fine, it works again, that's the most important part 😄

Sign up for freeto 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.

2 participants

@heidezomp@alexnask