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

[Task]: Replace Request::get with explicit input sources #874

Merged
merged 4 commits into from
Aug 26, 2024

Conversation

kingjia90
Copy link
Contributor

@kingjia90 kingjia90 commented Aug 19, 2024

Resolves #679
Related pimcore/pimcore#14138

Copy link

@mattamon
Copy link
Contributor

@kingjia90 What does it need to be finished? Could we already merge it?

Copy link
Contributor

@mcop1 mcop1 left a comment

Choose a reason for hiding this comment

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

Do we also need to update

$uri = $event->getRequest()->getMethod();
&
$uri = $event->getRequest()->getMethod();
?

@kingjia90
Copy link
Contributor Author

Do we also need to update

$uri = $event->getRequest()->getMethod();

&

$uri = $event->getRequest()->getMethod();

?

no needed because it's not marked as internal
https://github.com/symfony/http-foundation/blob/f602d5c17d1fa02f8019ace2687d9d136b7f4a1a/Request.php#L1125-L1138
and itself seems using the right explicit source approach
https://github.com/symfony/http-foundation/blob/f602d5c17d1fa02f8019ace2687d9d136b7f4a1a/Request.php#L1153

@mcop1
Copy link
Contributor

mcop1 commented Aug 26, 2024

Ok, then it looks good to me, thanks :)

@mcop1 mcop1 merged commit b6433ed into 1.x Aug 26, 2024
10 checks passed
@mcop1 mcop1 deleted the request-explicit-source branch August 26, 2024 09:55
@github-actions github-actions bot locked and limited conversation to collaborators Aug 26, 2024
@kingjia90 kingjia90 modified the milestones: 1.9.0, 1.8.0 Aug 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace Request::get with explicit input sources
3 participants