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

Split the time crate #350

Open
jhpratt opened this issue Sep 19, 2021 · 2 comments
Open

Split the time crate #350

jhpratt opened this issue Sep 19, 2021 · 2 comments
Assignees
Labels
C-blocked Category: blocked by another (possibly upstream) issue C-cleanup Category: cleanup of existing code E-medium Some experience needed.

Comments

@jhpratt
Copy link
Member

jhpratt commented Sep 19, 2021

The main time crate can be moved to time-core, while the formatting and parsing mechanism (including parsing format descriptions) can be moved to the time-formatting crate. At that point, the time crate could re-export everything from time-core, time-formatting, and time-macros, behind feature gates as appropriate. Implementations of internal helper traits in time-macros can be accomplished via a newtype.

The primary advantage of doing this is that the time-macros crate will no longer have to duplicate logic for a sizable number of items. Thus this change in structure will be an overall reduction in code. I believe that it will also improve compile-times, although this is already decent. Certainly the largest advantage is that because of the deduplication the macros will always be in sync with the core crate. This ensures that something can't exist in one and not the other.

These sub-crates, like time-macros, will not be intended for direct usage and should be considered an unstable implementation detail.

The time-formatting crate is blocked on rust-lang/cargo#8832; interaction between features would be far from ideal until this is stabilized (for six months per MSRV guarantees). Another blocker that I don't believe there is an issue for — if it's even desired — is having #[doc(cfg(…))] visible on re-exports. Without this a user may think that an API is available when it isn't. I'm guaranteeing that the feature flags in time, time-core, etc. all match but the compiler is unaware of this and thus removes the annotation in its entirety.

@jhpratt jhpratt added C-cleanup Category: cleanup of existing code E-medium Some experience needed. C-blocked Category: blocked by another (possibly upstream) issue labels Sep 19, 2021
@jhpratt jhpratt self-assigned this Sep 19, 2021
@jhpratt
Copy link
Member Author

jhpratt commented Jan 28, 2022

The primary blocker of weak dependencies will land in Rust 1.60, reaching MSRV on 2022-10-07. At some point before then I'll investigate the #[doc(cfg(…))] on re-exports and the potential for cyclical dependencies.

@jhpratt jhpratt removed the C-blocked Category: blocked by another (possibly upstream) issue label Jan 28, 2022
@jhpratt
Copy link
Member Author

jhpratt commented Aug 11, 2022

Looking into the #[doc(cfg(…))] story. Unfortunately it works the way I expected, which functionally means that the annotations are not visible because the re-exporting crate doesn't have anything behind a gate. This is inherent to the way workspaces work, so I don't see this changing any time soon.

For what it's worth, I may still go ahead and just hack some raw HTML into the definitions to simulate the existing annotations. The improvement in code structure may make it worthwhile.

@jhpratt jhpratt added the C-blocked Category: blocked by another (possibly upstream) issue label Feb 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-blocked Category: blocked by another (possibly upstream) issue C-cleanup Category: cleanup of existing code E-medium Some experience needed.
Projects
None yet
Development

No branches or pull requests

1 participant