8000 Move UriSigner from HttpKernel to HttpFoundation package by alexander-schranz · Pull Request #50392 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Move UriSigner from HttpKernel to HttpFoundation package #50392

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
Merged
merged 1 commit into from
Oct 2, 2023
Merged

Move UriSigner from HttpKernel to HttpFoundation package #50392

merged 1 commit into from
Oct 2, 2023

Conversation

alexander-schranz
Copy link
Contributor
@alexander-schranz alexander-schranz commented May 22, 2023 •
edited by derrabus
Loading
Q A
Branch? 6.4
Bug fix? no
New feature? yes
Deprecations? yes
Tickets Fix #50384
License MIT
Doc PR TODO

Move UriSigner from HttpKernel to HttpFoundation package as discussed in #50384.

@carsonbot carsonbot added this to the 6.3 milestone May 22, 2023
@carsonbot
Copy link

Hey!

Thanks for your PR. You are targeting branch "6.3" but it seems your PR description refers to branch "6.3 for features / 5.4 or 6.2 for bug fixes".
Could you update the PR description or change target branch? This helps core maintainers a lot.

Cheers!

Carsonbot

@alexander-schranz alexander-schranz force-pushed the feature/uri-signer-http-foundation branch 2 times, most recently from 59f312d to 93a5575 Compare May 22, 2023 23:51
nicolas-grekas
nicolas-grekas reviewed May 23, 2023
View reviewed changes
@nicolas-grekas nicolas-grekas modified the milestones: 6.3, 6.4 May 23, 2023
@alexander-schranz alexander-schranz mentioned this pull request Jul 10, 2023
Closed
@fabpot
Copy link
Member
fabpot commented Aug 4, 2023

@alexander-schranz Do you have time to finish this PR?

@alexander-schranz alexander-schranz force-pushed the feature/uri-signer-http-foundation branch 2 times, most recently from 6691053 to a3296c8 Compare August 4, 2023 10:31
@alexander-schranz
Copy link
Contributor Author

The fabbot seems to return a false positive currently:

Bildschirmfoto 2023-08-04 um 12 52 13

@alexander-schranz
Copy link
Contributor Author

If I see it correctly all failing tests are related to current 6.4 branch and not the pull request. So I hope I tackled all parts.

@alexander-schranz alexander-schranz force-pushed the feature/uri-signer-http-foundation branch from 2b93ab5 to aa2e016 Compare August 13, 2023 11:41
@alexander-schranz
Copy link
Contributor Author

@wouterj thx for the review, I hope I didn't miss something but it should be all tackled.

wouterj
wouterj approved these changes Aug 22, 2023
View reviewed changes
Copy link
Member
@wouterj wouterj left a comment

Choose a reason for hiding this comment

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

Thanks, just a few things were missed. But other than that, this looks good to go from my side!

@fabpot
Copy link
Member
fabpot commented Oct 1, 2023

@alexander-schranz Do you have time to finish this PR?

@wouterj
Copy link
Member
wouterj commented Oct 1, 2023

If not, please tell us and we (I) will force push the final tweaks to this PR so we can get it in 6.4/7.0 🙂

@alexander-schranz
Copy link
Contributor Author

@wouterj currently on vacation feel free to force push. Else I can finish it also in two weeks after my vacation.

@wouterj wouterj force-pushed the feature/uri-signer-http-foundation branch from 6831063 to 2caaef8 Compare October 2, 2023 12:22
@wouterj
Copy link
Member
wouterj commented Oct 2, 2023

Enjoy your holiday!

This PR should be ready now.

@fabpot
Copy link
Member
fabpot commented 8000 Oct 2, 2023

Thank you @alexander-schranz.

@fabpot fabpot mentioned this pull request Oct 2, 2023
Closed
@fabpot fabpot merged commit f7bfffa into symfony:6.4 Oct 2, 2023
This was referenced Oct 21, 2023
Merged
Merged
@alexander-schranz alexander-schranz deleted the feature/uri-signer-http-foundation branch November 6, 2023 06:46
@ausi ausi mentioned this pull request Nov 29, 2023
Merged
nicolas-grekas added a commit that referenced this pull request Nov 29, 2023
@nicolas-grekas
minor #52798 Fix legacy class palceholder definitions for static anal…
9144962 
…ysis (ausi)

This PR was squashed before being merged into the 6.4 branch.

Discussion
----------

Fix legacy class palceholder definitions for static analysis

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        |
| License       | MIT

In #50392 the `UriSigner` class was moved from HttpKernel to HttpFoundation. The empty definition `class UriSigner {}` causes many issues with static analysis tools like PHPStan.

As this code is wrapped in an `if (false)` changing it to `class UriSigner extends HttpFoundationUriSigner` should not cause any behavior change while fixing most static analysis issues.

Commits
-------

0f230fe Fix legacy class palceholder definitions for static analysis
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas left review comments

@HypeMC HypeMC HypeMC left revie 628C w comments

@fabpot fabpot fabpot approved these changes

@wouterj wouterj wouterj approved these changes

Assignees
No one assigned
Labels
Deprecation Feature Status: Reviewed
Projects
None yet
Milestone
6.4
Development

Successfully merging this pull request may close these issues.

[HttpKernel] Move UriSigner to Http Foundation
6 participants
@alexander-schranz @carsonbot @fabpot @wouterj @nicolas-grekas @HypeMC
0