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

Improve content type bundle's configuration. #260

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

bangpound
Copy link
Contributor

This branch is still a work in progress and will probably be rebased soon.

I'm rewriting SculpinContentTypesExtension as an exercise in becoming more acquainted with it. The outcomes of this work are:

  • Services are defined as abstract in services.xml and then concrete implementations are added in the extension for each configured type.
  • The default configuration for Sculpin (which provides the post type) moves out of the kernel and into the Content Type bundle.
  • Configuration class produces all the default values so the Extension can assume they are present.
  • Services are explicitly named in the Extension class instead of being calculated based on the return value of a method.
  • New compiler pass FilterPass adds tagged filter services to the ChainFilter for each type.

What's not great about this pull request:

  • More lines of code! The relationship between services.xml and the extension is still a bit uncertain. Most of the services are private and abstract and their configuration is incomplete. If more can be done in the configuration file, perhaps less needs to be done in the extension.
  • I made a lot of private static methods in the extension so I could more easily recognize the different parts and their dependencies.

What I still want to do:

  • Use a factory service similar to Symfony's service component to produce the content type services.
  • Rewrite FilterPass because it's weird and messy.
  • Whatever you think might be useful!

@bangpound bangpound force-pushed the feature/content-type-config branch 3 times, most recently from 28a450c to 26cb5d8 Compare February 28, 2016 00:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant