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

refactor: eliminate the deprecation warning #195

Merged
merged 1 commit into from
Mar 31, 2024
Merged

Conversation

yingmanwumen
Copy link
Contributor

chrono has deprecated some methods of NaiveDataTime: https://docs.rs/chrono/latest/chrono/naive/struct.NaiveDateTime.html#method.from_timestamp I apply cargo clippy to correct it, and all tests are passed locally.

What problem does this PR solve?

There're some compiling warnings related to some deprecated methods in chrono::NaiveDataTime.

What is changed and how it works?

Use the recommanded equal methods to replace the old ones.

Code changes

  • Has Rust code change
  • Has CI related scripts change

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

I ran cargo test locally in my machine and all of the testcases passed:

test result: ok. 99 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.60s

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Note for reviewer

It's a tiny refactor that doesn't alter the behavior of other codes.

chrono has deprecated some methods of NaiveDataTime:
https://docs.rs/chrono/latest/chrono/naive/struct.NaiveDateTime.html#method.from_timestamp
I apply `cargo clippy` to correct it, and all tests are passed locally.
@KKould KKould requested review from KKould and crwen March 31, 2024 10:32
@KKould KKould added the invalid This doesn't seem right label Mar 31, 2024
Copy link
Member

@KKould KKould left a comment

Choose a reason for hiding this comment

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

In fact, I originally wanted to classify this issue into #182 because it needs to support time zones.
BTW, LGTM

@KKould KKould merged commit f68904c into KipData:main Mar 31, 2024
3 checks passed
@KKould
Copy link
Member

KKould commented Mar 31, 2024

Thanks for your contribution.🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants