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

Update framework provided property annotations #17249

Merged
merged 2 commits into from
Aug 22, 2023
Merged

Update framework provided property annotations #17249

merged 2 commits into from
Aug 22, 2023

Conversation

markstory
Copy link
Member

Update the components we annotate in Controller to no longer include deprecated components and include the CheckHttpCache component which was omitted.

Fixes #17247

Update the components we annotate in `Controller` to no longer include
deprecated components and include the `CheckHttpCache` component which
was omitted.

Fixes #17247
@markstory markstory added this to the 4.4.18 milestone Aug 22, 2023
@dereuromark
Copy link
Member

Dont we need those for static analyzers still? Deprecated doesnt mean unused imo.

@markstory
Copy link
Member Author

Dont we need those for static analyzers still? Deprecated doesnt mean unused imo.

I don't think we need it for our static-analysis. If applications are using the deprecated components they could add the necessary annotations.

@LordSimal
Copy link
Contributor

LordSimal commented Aug 22, 2023

Imho this should stay as it is because

  1. the components are still there and "valid" working components
  2. the problem in Authentication Object deprecated #17247 would have been fixed automatically if the user used the ide-helper annotate command to automatically add the correct $Authentication dynamic property to the AppController (and therefore all subsequeunt controllers) instead of mixing up $this->Auth with $this->Authentication (yes, its stupid but that's just the in-between time period we now live in moving away from the old AuthComponent to the new Auth plugins)

if we instead really remove those dynamic properties we could of course ignore the now newly occuring stan error but this doesn't feel right to me

@markstory
Copy link
Member Author

Ok, let's leave the components as they are and only add HttpCacheCheck.

Because these comoponents still exist and still work fine we don't need
to remove them.
@dereuromark dereuromark merged commit b58978b into 4.x Aug 22, 2023
12 of 13 checks passed
@dereuromark dereuromark deleted the issue-17247 branch August 22, 2023 22:19
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.

Authentication Object deprecated
3 participants