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

[11.x] Fix deprecation warning caused by Carbon 3.2 #50813

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

jackwh
Copy link
Contributor

@jackwh jackwh commented Mar 28, 2024

Since running composer update today, I noticed deprecation errors being logged for every Laravel request to my app:

Use the method UTCInRealSeconds instead to make it more explicit about what it does. On next major version, "float" prefix will be removed (as all diff are now returning floating numbers) and "Real" methods will be removed in favor of "UTC" because what it actually does is to convert both dates to UTC timezone before comparison, while by default it does it only if both dates don't have exactly the same timezone (Note: 2 timezones with the same offset but different names are considered different as it's not safe to assume they will always have the same offset). in /Users/jack/Sites/site/vendor/nesbot/carbon/src/Carbon/Traits/Date.php on line 2627

I've replaced diffInRealSeconds with diffInSeconds to fix this, to retain compatibility with the Carbon v2 dependency as well.

Here's the Carbon commit which caused this, released in 3.2 yesterday.

@driesvints driesvints changed the title Fix deprecation warning caused by Carbon 3.2 [11.x] Fix deprecation warning caused by Carbon 3.2 Mar 28, 2024
@driesvints
Copy link
Member

cc @kylekatarnls

@kylekatarnls
Copy link
Contributor

Hello, sorry the deprecation is misformed, it should be written diffInUTCSeconds, but with any unit smaller than day, timezone has no impact, so indeed diffInSeconds is a good replacement.

@kylekatarnls
Copy link
Contributor

kylekatarnls commented Mar 28, 2024

As a side note, "Real" alias methods will remain available for the whole life of 3.x

@jackwh
Copy link
Contributor Author

jackwh commented Mar 28, 2024

@driesvints Can this be tagged for release today, or should we downgrade Carbon temporarily? I imagine it'll fill up a lot of logs in the meantime as it hits on every request.

@driesvints
Copy link
Member

@jackwh not sure why deprecations are hitting you in production?

@jackwh
Copy link
Contributor Author

jackwh commented Mar 28, 2024

@driesvints New Relic still processes deprecations in production, even when disabled in Laravel. And of course it's just an annoyance in local development as well, with tools like Ray becoming nearly unusable.

@taylorotwell taylorotwell merged commit 71561e7 into laravel:11.x Mar 28, 2024
30 checks passed
@kylekatarnls
Copy link
Contributor

And of course it's just an annoyance in local development as well, with tools like Ray becoming nearly unusable.

Sorry I don't get this point neither, you have local development tool that check runtime-deprecation notices? 🤔

I would reconsider the plan for deprecation if needed. But I would need to understand exactly which tool complains about what.

Deprecation is a way to offer a notice ahead of later changes. So the whole point is that it's actually not an emergency to be changed in the minute. If a third-party tool defeats this purpose, I think it's worth to check if it has options (such as way of acknowledging a particular deprecation so not to be raised again). If it has not, it would be relevant to open a discussions on with the maintainers of tools.

@jackwh
Copy link
Contributor Author

jackwh commented Apr 2, 2024

@kylekatarnls I agree with you and have no issue with how Carbon (or any other dependency) raises deprecation notices 👍 I only suggested releasing this sooner as I know Laravel values a good developer experience, and given this single isolated call to a deprecated function logs on every framework request, it would be nice if we didn't have to work around it for a week before the next release. Thanks for your hard work on Carbon ❤️

@driesvints
Copy link
Member

We released this last Thursday btw 👍

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