Skip to content

[FrameworkBundle] Fix merge error with ErrorLoggerCompilerPass #52023

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

[FrameworkBundle] Fix merge error with ErrorLoggerCompilerPass #52023

nicolas-grekas merged 1 commit into from
Oct 13, 2023

Conversation

@PhilETaylor
Copy link
Contributor

@PhilETaylor PhilETaylor commented Oct 12, 2023 •
edited
Loading

Q A
Branch? 6.3
Bug fix? no
New feature? no
Deprecations? no
Tickets Fix #...
License MIT

@nicolas-grekas @MatTheCat does this make sense? If I change these 2 lines in 6.4 then my app loads fine.

relates to #52009 (comment)

This comment from @HypeMC #52009 (comment) guided my debugging

or am I barking up the wrong tree?

@carsonbot carsonbot added this to the 6.3 milestone Oct 12, 2023
@PhilETaylor PhilETaylor mentioned this pull request Oct 12, 2023
Merged
@HypeMC
Copy link
Member

HypeMC commented Oct 12, 2023

or am I barking up the wrong tree?

You're not, this definitively needs to be changed on >=6.3 cause of #49275

@PhilETaylor
Copy link
Contributor Author

PhilETaylor commented Oct 12, 2023

Thanks for confirming. Ive next to zero experience in compiler pass layers, but love to debug when I should be doing productive things haha

Hope this PR helps out Nicolas.

@OskarStark OskarStark changed the title [FrameworkBundle ]Fix merge error with ErrorLoggerCompilerPass [FrameworkBundle] Fix merge error with ErrorLoggerCompilerPass Oct 12, 2023
MatTheCat
MatTheCat approved these changes Oct 13, 2023
View reviewed changes
Copy link
Contributor

@MatTheCat MatTheCat left a comment

Choose a reason for hiding this comment

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

Yep seems good to me 👌

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Oct 13, 2023

Thank you @PhilETaylor.

@nicolas-grekas nicolas-grekas merged commit 39a56d3 into symfony:6.3 Oct 13, 2023
@xabbuh xabbuh mentioned this pull request Oct 13, 2023
Closed
@PhilETaylor PhilETaylor deleted the patch-13 branch October 13, 2023 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@MatTheCat MatTheCat MatTheCat approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Bug FrameworkBundle Status: Reviewed

Projects

None yet

Milestone

6.3

Development

Successfully merging this pull request may close these issues.

6 participants

@PhilETaylor @HypeMC @nicolas-grekas @MatTheCat @derrabus @carsonbot