Skip to content

Conversation

@GwendolenLynch
Copy link
Contributor

@GwendolenLynch GwendolenLynch commented Dec 7, 2017

This enables the services to be DIed on type hint:

public function myAction(Request $request, HandlerFactory $handlerFactory): Response

Fixes #48

This enables the services to be DIed on type hint:

`public function myAction(Request $request, HandlerFactory $handlerFactory): Response`
@linaori
Copy link
Contributor

linaori commented Dec 7, 2017

What if we do it the other way around, alias the old definitions and make the FQCN leading? I would also like to see an alias for the interface, pointing towards the implementation, this makes it possible to easily decorate (think logging/timeline for example).

Additionally, can you add a test to verify if the autowiring behavior works in properly in 3.3+? This is merely to verify that the service config is written in a way it won't fail after updating.

Edit: I've added a new issue specifically for the aliases (the other one is autoconfigure for the tags).

@GwendolenLynch
Copy link
Contributor Author

GwendolenLynch commented Dec 7, 2017

Happy to switch them around, I was unsure of the preferred approach wrt BC.

All my work is on 4.0.x, so yep, they're working there. But happy to run up a 3.4 site to check as well if you think it relevant.

@linaori
Copy link
Contributor

linaori commented Dec 7, 2017

Regarding tests I mean this one: https://github.com/hostnet/form-handler-bundle/blob/master/test/Functional/Fixtures/config/config_33.yml

Can you add a public class here that requires in the constructor (or optionally arguments for controller if you like):

  • FormProviderInterface
  • SimpleFormProvider
  • HandlerFactoryInterface
  • HandlerFactory

This way we can keep the guarantee that this works.

@GwendolenLynch
Copy link
Contributor Author

Should be GTG now! Thanks for your patience.

@linaori
Copy link
Contributor

linaori commented Dec 9, 2017

Can you also add aliases to the interfaces? Other than that, looks good to me!

@GwendolenLynch
Copy link
Contributor Author

Gedaan!

@hboomsma hboomsma merged commit ffa8079 into hostnet:master Dec 12, 2017
@hboomsma
Copy link
Contributor

hboomsma commented Dec 12, 2017

I'll ask @iltar to draft a release after we discussed #46

@GwendolenLynch GwendolenLynch deleted the service-aliases branch December 12, 2017 06:46
@GwendolenLynch
Copy link
Contributor Author

Much appreciated. Thanks for a great bundle 👍

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.

4 participants