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

Change RecordLocation members lifetime - log 0.4 support #171

Merged
merged 2 commits into from
Jun 21, 2018
Merged

Change RecordLocation members lifetime - log 0.4 support #171

merged 2 commits into from
Jun 21, 2018

Conversation

jacekchmiel
Copy link
Contributor

The file, function, module str lifetimes was changed to not require
'static lifetimes. (As log 0.4 introduced for better interoperability).

The file, function, module str lifetimes was changed to not require
'static lifetimes. (As log 0.4 introduced for better interoperability).
@jacekchmiel
Copy link
Contributor Author

This is a part of work connected with:
slog-rs/async#8
slog-rs/stdlog#5
slog-rs/envlogger#6

@jacekchmiel
Copy link
Contributor Author

I will require some guidance on how to manage Cargo files for such distributed change.

@@ -52,3 +52,6 @@ slog-async = "2"

[package.metadata.docs.rs]
features = ["std", "nested-values", "dynamic-keys"]

[patch.crates-io]
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will have to go if this is to be landed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I know, just wanted to make whole code build.

@@ -2265,17 +2265,17 @@ fn level_from_str() {
// {{{ Record
#[doc(hidden)]
#[derive(Clone, Copy)]
pub struct RecordLocation {
pub struct RecordLocation<'a> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Well, this is going to be a breaking change. :/

@dpc
Copy link
Collaborator

dpc commented Jun 20, 2018

I really appreciate your time. I guess there is no way around the fact that this is a breaking change. I guess it will have to be slog 3.x.y .

And if we are to do v3, then I have bunch of stuff that I would like to change: https://github.com/slog-rs/slog/wiki/Things-to-make-different-next-time-around-(slog-v3%3F)

@dpc
Copy link
Collaborator

dpc commented Jun 20, 2018

On top of it, there is a discussion about having structured logging in log and it would be very much like logging in slog. Also log v1

So given all that, maybe it would be better to wait for log v1, and then release slog v3 on top of it?

@jacekchmiel
Copy link
Contributor Author

If log v1 is really close on the horizon, it would be indeed worth waiting for that. In the meantime I'll keep my forks (probably using them) - they may come handy at some point.

By the way, what will be the point of slog when log v1 with structured logging lands? Just to support current users and allow them to integrate with log v1?

@dpc
Copy link
Collaborator

dpc commented Jun 20, 2018

Contextual logging (pasing logging around with their associated KVs) is something that log maintainers are not that excited about, so I think log would become a contextual logging lib on top of log.

@Techcable
Copy link
Member

Techcable commented Jun 20, 2018

@dpc
So log won't include support for contextual Loggers and we'll still have to use slog for that?
I think the ergonomic cost of contextual logging is very low. It's extremely popular in Python, Java, and C# so it's not like its a crazy or unergonomic idea.
Too bad it won't be included in the official log library 😞

Still, structured logging is an important first step and is quite possibly more important due to the availability of slog-scope.

@dpc
Copy link
Collaborator

dpc commented Jun 21, 2018

From my understanding, developers working on log are not convinced about contextual logging and passing Loggers around. I'll try to make some provisions to allow this to happen via extensions compatible with log. You can always look into existing issues on log github repo: https://github.com/rust-lang-nursery/log/issues

@dpc dpc merged commit 36893e2 into slog-rs:master Jun 21, 2018
@jacekchmiel
Copy link
Contributor Author

@dpc didn't you want to close this instead of merging?

@dpc
Copy link
Collaborator

dpc commented Jun 21, 2018

I've decided to toy around with all of this stuff. I've created a v2 branch for stable version.

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.

3 participants