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

Replace humantime with cyborgtime #351

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

oherrala
Copy link
Contributor

@oherrala oherrala commented Mar 8, 2025

humantime seems to be unmaintained and cyborgtime is fork of it. This change shouldn't affect end users. E.g. "humantime" feature is kept as is.

I also introduced "cyborgtime" feature that currently works same as "humantime".

Ref. rustsec/advisory-db#2249

humantime seems to be unmaintained and cyborgtime is fork of it. This
change shouldn't affect end users. E.g. "humantime" feature is kept as
is.

I also introduced "cyborgtime" feature that currently works same as
"humantime".
@oherrala oherrala marked this pull request as draft March 8, 2025 15:31
@coveralls
Copy link

Pull Request Test Coverage Report for Build 13739014629

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 43.643%

Totals Coverage Status
Change from base Build 13441892276: 0.0%
Covered Lines: 254
Relevant Lines: 582

💛 - Coveralls

@BurntSushi
Copy link
Contributor

(Disclosure: I'm the author of Jiff.)

This kinda looks like you're replacing one unmaintained dependency with another. cyborgtime was forked ~2 years ago, had a few commits and then hasn't seen any activity since. It doesn't seem like the right play here?

On the other hand, bringing in something like Jiff (or even chrono or time) seems a little heavyweight for this. I'm not sure there is really much of a middle ground other than sticking with humantime. Then again, I think this is optional (albeit enabled by default), so maybe it's fine.

@epage
Copy link
Contributor

epage commented Mar 8, 2025

I'd evalwate switching to jiff. While its heavier, for the use case, I figure its more likely to be used by other dependencies in the future.

I'd keep the feature name as humantime as that also works as a description of behavior and not just a crate name.

In the future, maybe we should make this feature non-default.

@oherrala
Copy link
Contributor Author

oherrala commented Mar 8, 2025

@epage @BurntSushi I made a jiff version in #352.

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.

4 participants