Stabilize kv_unstable #436

Closed
opened 2021-01-07 23:14:31 -06:00 by KodrAus · 9 comments
KodrAus commented 2021-01-07 23:14:31 -06:00 (Migrated from github.com)

This is part of #328

I think we've explored enough of this design space now and the API has been in use within the tide project for some time now with their own kv-capable logging macro.

By stabilizing just the backend (non-macro-supported) kv module we can let users depend on this with confidence so that projects like tracing can also utilize it.

Migration path

The kv API will remain an opt-in optional API. All features that currently begin with kv_unstable will be changed to kv. We can keep these old unstable features around for a while if we need.

Any other changes that are needed should be included here.

Steps

  • Public API review of the kv module
  • Stabilize sval
  • Stabilize value-bag
  • Documentation and examples

cc @yoshuawuyts

This is part of #328 I think we've explored enough of this design space now and the API has been in use within the `tide` project for some time now with their own kv-capable logging macro. By stabilizing just the backend (non-macro-supported) `kv` module we can let users depend on this with confidence so that projects like `tracing` can also utilize it. # Migration path The `kv` API will remain an opt-in optional API. All features that currently begin with `kv_unstable` will be changed to `kv`. We can keep these old unstable features around for a while if we need. Any other changes that are needed should be included here. # Steps - [x] Public API review of the `kv` module - [x] Stabilize [`sval`](https://github.com/sval-rs/sval) - [x] Stabilize [`value-bag`](https://github.com/sval-rs/value-bag) - [ ] Documentation and examples cc @yoshuawuyts
Thomasdezeeuw commented 2021-01-08 06:32:14 -06:00 (Migrated from github.com)

I don't know what the plan is for the kv_unstable_std feature, but can that be removed? Surely most users mean to enable kv_unstable_std if std and kv are enabled.

I don't know what the plan is for the `kv_unstable_std` feature, but can that be removed? Surely most users mean to enable `kv_unstable_std` if `std` and `kv` are enabled.
KodrAus commented 2021-01-08 06:56:58 -06:00 (Migrated from github.com)

@Thomasdezeeuw unfortunately it’s a limitation of Cargo features. Since it’s not possible to activate a feature of a dependency when two unrelated features are active we need one that covers them both.

@Thomasdezeeuw unfortunately it’s a limitation of Cargo features. Since it’s not possible to activate a feature of a dependency when two unrelated features are active we need one that covers them both.
Thomasdezeeuw commented 2021-01-08 07:00:25 -06:00 (Migrated from github.com)

@KodrAus in the code you can use #[cfg(all(feature = "kv", feature = "std"))] in stead of #[cfg(feature = "kv_unstable_std")], removing the kv_unstable_std feature.

@KodrAus in the code you can use `#[cfg(all(feature = "kv", feature = "std"))]` in stead of `#[cfg(feature = "kv_unstable_std")]`, removing the `kv_unstable_std` feature.
KodrAus commented 2021-01-08 17:40:19 -06:00 (Migrated from github.com)

@Thomasdezeeuw we can do that in code, but we also have dependencies that need their own std features enabled too.

@Thomasdezeeuw we can do that in code, but we also have dependencies that need their own `std` features enabled too.
Thomasdezeeuw commented 2021-12-04 06:11:12 -06:00 (Migrated from github.com)

@KodrAus could you make another release to try the new macro syntax?

@KodrAus could you make another release to try the new macro syntax?
KodrAus commented 2021-12-07 03:39:20 -06:00 (Migrated from github.com)

@Thomasdezeeuw Will do! I'll get the ball rolling on this now.

@Thomasdezeeuw Will do! I'll get the ball rolling on this now.
Thomasdezeeuw commented 2022-02-21 05:32:39 -06:00 (Migrated from github.com)

@KodrAus any update on a release? We've been using the git version without issues for a while (including the new macro syntax).

@KodrAus any update on a release? We've been using the git version without issues for a while (including the new macro syntax).
KodrAus commented 2022-02-22 16:23:58 -06:00 (Migrated from github.com)

@Thomasdezeeuw I've been waiting for some feedback from the wider project on https://github.com/rust-lang/log/issues/343#issuecomment-1002851191, but it's been a bit quiet. I'm pretty confident in the direction we're going though and that we're not heading down a path we'll regret later so will push ahead.

@Thomasdezeeuw I've been waiting for some feedback from the wider project on https://github.com/rust-lang/log/issues/343#issuecomment-1002851191, but it's been a bit quiet. I'm pretty confident in the direction we're going though and that we're not heading down a path we'll regret later so will push ahead.
KodrAus commented 2022-02-22 16:44:44 -06:00 (Migrated from github.com)

I've opened #485 to get a release out the door. I'm just going to run through the diff, but if you had a chance to take a look too @Thomasdezeeuw that would be much appreciated! 🙇

I've opened #485 to get a release out the door. I'm just going to run through the diff, but if you had a chance to take a look too @Thomasdezeeuw that would be much appreciated! :bow:
Sign in to join this conversation.
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-rust-lang/log#436
No description provided.