Skip to content

Show compact way to implement a compiler pass #19210

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
Closed
wants to merge 1 commit into from
Closed

Show compact way to implement a compiler pass #19210

wants to merge 1 commit into from

Conversation

tacman
Copy link
Contributor

@tacman tacman commented Dec 4, 2023

AbstractBundle is great. I love not having to create a separate CompilerPass class.

xabbuh
xabbuh reviewed Dec 4, 2023
View reviewed changes
service_container/compiler_passes.rst
@@ -87,6 +87,17 @@ method in the extension)::
}
}

:doc:Implement process() directly in the bundle definition by extending AbstractBundle
Copy link
Member

Choose a reason for hiding this comment

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

I would not add a new paragraph here but instead let the MyBundle class from the example above also implement the CompilerPassInterface and the process() method and add a comment inside this method that a bundle can be a compiler pass itself (we already mention process() in the text above).

@OskarStark
Copy link
Contributor

Would you like to finish your PR?

@javiereguiluz javiereguiluz mentioned this pull request Oct 31, 2024
Merged
@javiereguiluz
Copy link
Member

I like the idea of showing the more compact way first. So, I followed @xabbuh's recommendation and created #20365 to fix this and other issues in this article.

So, let's close this PR in favor of the other one. Thanks.

javiereguiluz added a commit that referenced this pull request Nov 6, 2024
@javiereguiluz
minor #20365 [DependencyInjection] Update the article about compiler …
d1a1278 
…passes (javiereguiluz)

This PR was merged into the 6.4 branch.

Discussion
----------

[DependencyInjection] Update the article about compiler passes

This is an alternative to #19210.

Commits
-------

a18cae1 [DependencyInjection] Update the article about compiler passes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xabbuh xabbuh xabbuh left review comments

Assignees
No one assigned
Labels
Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@tacman @OskarStark @javiereguiluz @xabbuh @carsonbot