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.

Sign up for GitHub

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

Jump to bottom

Fix chrono deprecation warning #4785

Merged
merged 2 commits into from
Dec 11, 2024
Merged

Fix chrono deprecation warning #4785

merged 2 commits into from
Dec 11, 2024

Conversation

bschoenmaeckers
Copy link
Contributor

No description provided.

@Icxolu Icxolu added the CI-skip-changelog Skip checking changelog entry label Dec 10, 2024
@Icxolu Icxolu added this pull request to the merge queue Dec 10, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 10, 2024
@Icxolu
Copy link
Contributor

Icxolu commented Dec 10, 2024

I think we need to bump chronos minimum version slightly for this.

@bschoenmaeckers
Copy link
Contributor Author

I think we need to bump chronos minimum version slightly for this.

Is that desirable because we only use this function in our test suite?

@ngoldbaum
Copy link
Contributor

The merge queue CI failed, see https://github.com/PyO3/pyo3/actions/runs/12264475737/job/34218435360#step:8:340

The MIN and MAX constants were just added in the most recent version of chrono, so I don't think bumping the minimum version is correct.

Maybe we should open an issue in chrono asking for a way to spell this that works on older chrono versions.

@ngoldbaum ngoldbaum mentioned this pull request Dec 11, 2024
Closed
@Icxolu
Copy link
Contributor

Icxolu commented Dec 11, 2024

Ah, I didn't realize that they are only available in the lastest patch. In that case it really seems unfortunate to only support that version of chrono... Maybe we just #[allow(deprecated)] for now and bump the minimum version in the future?

@ngoldbaum
Copy link
Contributor

[allow(deprecated)] is a bit of a blunt hammer but I think it's the most reasonable thing to do given that cargo test doesn't support runtime skips.

@ngoldbaum
Copy link
Contributor

@bschoenmaeckers hope you don't mind me pushing to your branch, I'd like to get CI unblocked today :)

@Icxolu Icxolu enabled auto-merge December 11, 2024 17:14
@bschoenmaeckers
Copy link
Contributor Author

@bschoenmaeckers hope you don't mind me pushing to your branch, I'd like to get CI unblocked today :)

Go ahead!

@Icxolu Icxolu added this pull request to the merge queue Dec 11, 2024
Merged via the queue into PyO3:main with commit 926f5cf Dec 11, 2024
46 checks passed
@bschoenmaeckers bschoenmaeckers deleted the fix-chrono branch December 11, 2024 21:44
davidhewitt pushed a commit that referenced this pull request Jan 3, 2025
@bschoenmaeckers @ngoldbaum @davidhewitt
Fix chrono deprecation warning (#4785)
5d1a058 
* Fix chrono deprecation warning

* use allow(deprecated) instead

---------

Co-authored-by: Nathan Goldbaum <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Icxolu Icxolu Icxolu approved these changes

Assignees
No one assigned
Labels
CI-skip-changelog Skip checking changelog entry
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bschoenmaeckers @Icxolu @ngoldbaum