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

[10.x] create "Deprecations" documentation #8590

Closed
wants to merge 1 commit into from

Conversation

browner12
Copy link
Contributor

In light of the recent comments on laravel/framework#42587 (comment), I thought it might be worthwhile to create some documentation on deprecations in the framework to make them more obvious to users who maybe aren't digging through the code and watching the commits all the time.

We have two "events" that are helpful to document:

  • code is deprecated
  • deprecated code is removed

We currently, as @driesvints pointed out, document in the Upgrade Guide when deprecated code is removed. We, however, don't currently document when code is deprecated. I don't think it makes sense to add this to the Upgrade Guide, as it could add a lot of noise, and is not immediately relevant to people performing upgrades. This is why I think having a separate page for it makes the most sense. Having this page will help avoid users getting blindsided in the upgrade process from removed code possibly requiring larger refactors.

I definitely still think the code is the best place to go as the source of truth for finding deprecated code, but I also kind of see the side of it never being explicitly presented to the masses.

This page will contain documentation on code or features that have been deprecated in the framework, possibly some notes on how to update, and possibly when the code will be removed.

this file will contain documentation on code or features that have been deprecated in the framework, possibly some notes on how to update, and possibly when the code will be removed.
@driesvints
Copy link
Member

I appreciate the thought @browner12 but I don't think it's maintainable for us to keep updating this every time something is deprecated. A @deprecated annotation is added to code when it's deprecated and most IDE's nowadays indicate that when you're browsing that code. I don't think an entire new section is warranted here.

@decadence
Copy link
Contributor

For example PhpStorm crosses out deprecated variable only if it's used in code somewhere but not if it's overridden property (like $dates which we don't use in methods). And then not in all cases.

@nanaya
Copy link
Contributor

nanaya commented Feb 17, 2023

Just adding a bit more, just @deprecated tag doesn't really mean much when there's no explanation on it - it is just gone for good or its functionality has been replaced by something else or some other reason for it being deprecated (unless if people are expected to dig through the commit/blame log to figure it out, that is).

@browner12
Copy link
Contributor Author

So here's a pretty good example of where documentation like this would help. I'm currently running through some Pint fixes on my newly updated Laravel 10 app. I ran Pint on my custom validation rules, and it made a couple changes, but I also decided to open them in the IDE just to take a look. Lo and behold, the Illuminate\Contracts\Validation\Rule interface has been deprecated.

I'm still using the passes() and message() methods, completely unaware there is a new way of writing custom validation rules. Let's say the interface gets removed in Laravel 10. Yes, that removal would probably be in the upgrade guide, and it probably wouldn't be too time consuming, but if we had deprecation documentation, I would be more likely to make the necessary adjustments beforehand in preparation.

@taylorotwell
Copy link
Member

I think one thing that keeps me from merging this is for Laravel "deprecated" doesn't necessarily mean it will be removed in a future version. In my mind, it means something more like "legacy" or "old way of doing things" or "no longer recommended". I personally don't mind keeping "deprecated" things around forever if it doesn't give us any headache because it saves us having to make a breaking change.

@browner12
Copy link
Contributor Author

I personally don't mind keeping "deprecated" things around forever

My big concern with this is bloat and maintainability, but there's obviously a tradeoff here that you as the owner have to decide on, so I won't belabor this point.

"deprecated" doesn't necessarily mean it will be removed

@taylorotwell herein lies the main problem. You are introducing uncertainty by saying deprecated code may or may not be removed at a later time. If some deprecated code gets removed relatively close to before a new release it adds some additional burden to the upgrade process. By documenting these deprecations I'd argue it actually helps with BC, because it gives users a very large window of time to opt into making the adjustments if they so choose.

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.

5 participants