Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

[HttpKernel] Marked the class DebugHandlersListener as internal #39016

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
Dec 8, 2020
Merged

[HttpKernel] Marked the class DebugHandlersListener as internal #39016

merged 1 commit into from
Dec 8, 2020

Conversation

lyrixx
Copy link
Member

@lyrixx lyrixx commented Nov 6, 2020 •
edited
Loading

Q A
Branch? 5.x
Bug fix? no
New feature? no
Deprecations? no
Tickets -
License MIT
Doc PR -

@carsonbot carsonbot added this to the 4.4 milestone Nov 6, 2020
@lyrixx lyrixx force-pushed the error-handler-dead-code branch from f2a6294 to db06c07 Compare November 6, 2020 14:19
@lyrixx lyrixx changed the base branch from 4.4 to 5.x November 6, 2020 15:34
@lyrixx lyrixx force-pushed the error-handler-dead-code branch 2 times, most recently from b05be50 to 95aa265 Compare November 6, 2020 15:35
@lyrixx lyrixx modified the milestones: 4.4, 5.2 Nov 6, 2020
@lyrixx lyrixx force-pushed the error-handler-dead-code branch from 95aa265 to bc35a40 Compare November 6, 2020 15:38
@lyrixx lyrixx force-pushed the error-handler-dead-code branch from bc35a40 to 05d0686 Compare November 6, 2020 15:51
@nicolas-grekas nicolas-grekas modified the milestones: 5.2, 5.x Nov 6, 2020
derrabus
derrabus requested changes Nov 6, 2020
View reviewed changes
Copy link
Member

@derrabus derrabus left a comment

Choose a reason for hiding this comment

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

With this change alone, you create a constructor parameter that has to be always null and you still won't be able to remove it in Symfony 6.

What you need to do is remove the $fileLinkFormat parameter from the signature. This way, all following parameters are shifted left. If something that looks like a file link format is passed to $scope (which is now at the position where $fileLinkFormat previously was), you trigger the deprecation.

@lyrixx
Copy link
Member Author

lyrixx commented Nov 6, 2020

@derrabus What about the solution of @nicolas-grekas? It sounds simpler to me :)

@yceruto
Copy link
Member

yceruto commented Nov 6, 2020

the alternative is to make this class @internal since 5.3 and do what we want in v6...

I'm for this alternative 👍, this class looks really @internal.

@derrabus
Copy link
Member

derrabus commented Nov 6, 2020

the alternative is to make this class @internal since 5.3 and do what we want in v6...

I'm for this alternative 👍, this class looks really @internal.

I agree. In that case, we wouldn't need a deprecation layer at all, I guess.

@chalasr
Copy link
Member

chalasr commented Nov 17, 2020

@lyrixx caution, it seems that you pushed a commit under my name :)

@lyrixx lyrixx force-pushed the error-handler-dead-code branch from 05d0686 to f796fb3 Compare November 30, 2020 19:37
@lyrixx lyrixx changed the title [HttpKernel] Remove dead code in DebugHandlersListener [HttpKernel] Marked the class DebugHandlersListener as internal Nov 30, 2020
@lyrixx lyrixx force-pushed the error-handler-dead-code branch from f796fb3 to dfdedbd Compare November 30, 2020 19:37
@lyrixx
Copy link
Member Author

lyrixx commented Nov 30, 2020 •
edited
Loading

I updated the PR to mark the class Symfony\Component\HttpKernel\EventListener\DebugHandlersListener as internal

Should I put something in UPGRADE-6.0.md ?

@lyrixx lyrixx force-pushed the error-handler-dead-code branch from dfdedbd to 86ed073 Compare November 30, 2020 23:08
@lyrixx lyrixx force-pushed the error-handler-dead-code branch from 86ed073 to 69c9891 Compare December 5, 2020 16:44
@lyrixx
Copy link
Member Author

lyrixx commented Dec 8, 2020

Hello,

What is missing for this PR?

Note: I keep rebasing this PR because of a conflit on UPGRADE-5.3.md

@lyrixx lyrixx requested a review from nicolas-grekas December 8, 2020 10:29
@lyrixx lyrixx force-pushed the error-handler-dead-code branch from 69c9891 to e531d67 Compare December 8, 2020 10:29
@nicolas-grekas
Copy link
Member

Maybe add a changelog entry in the component?

@lyrixx lyrixx force-pushed the error-handler-dead-code branch from e531d67 to fca04c2 Compare December 8, 2020 13:10
@lyrixx
Copy link
Member Author

lyrixx commented Dec 8, 2020

I added a note in the CHANGELOG of HttpKernel Component

@nicolas-grekas nicolas-grekas force-pushed the error-handler-dead-code branch from fca04c2 to 1ee7e4c Compare December 8, 2020 13:18
@nicolas-grekas
Copy link
Member

Thank you @lyrixx.

@nicolas-grekas nicolas-grekas merged commit d07b951 into symfony:5.x Dec 8, 2020
@lyrixx lyrixx deleted the error-handler-dead-code branch December 8, 2020 16:05
@lyrixx lyrixx mentioned this pull request Jul 19, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derrabus derrabus derrabus requested changes

@yceruto yceruto yceruto left review comments

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

Assignees
No one assigned
Labels
Status: Reviewed
Projects
None yet
Milestone
5.4
Development

Successfully merging this pull request may close these issues.
6 participants
@lyrixx @yceruto @derrabus @chalasr @nicolas-grekas @carsonbot
Morty Proxy This is a proxified and sanitized view of the page, visit original site.