skrz / autowiring-bundle

Autowiring + automatic service discovery for Symfony DI container
MIT License
8 stars 6 forks source link

Tests added, small changes #2

Closed TomasVotruba closed 9 years ago

TomasVotruba commented 9 years ago

Ref #1

I don't understand that autoloading part (what for?) and property autowiring.

I've added plenty package design changes, like docs syntax highlighting, .gitignore fixes, Readme fixes etc. Feel free to ask if you don't understand or don't agree with anything your find.

Note: All I use and need is ctor autowiring. I don't use neither @autowired and setters in Nette. So I'll probably clean the package and own, smaller one.

jakubkulhan commented 9 years ago

Firstly, great work. I've added some comments to the diff.

Thank you for moving things around to match bundle standards!

TomasVotruba commented 9 years ago

@jakubkulhan Do you have some code sniffer rules to remove manual burden? They would be much helpful.

TomasVotruba commented 9 years ago

Tests finally fixed!

TomasVotruba commented 9 years ago

Thanks!

jakubkulhan commented 9 years ago

I had to add back SkrzAutowiringBundle::getContainerExtension(), otherwise Symfony threw exception regarding extension naming.

Also this big pull request finally pushed me to automate code style checking/fixing using PHPCS (currently only for internal projects, however same code style I used to reformat the merge). Thank you very much for that, and of course the code! :)

TomasVotruba commented 9 years ago

I don't understand the issue. What's the expected name and what's the real one? Renaming to match didn't work?

Great, I'm just integrating php-cs-fixer :). I Used phpcs before, but it took too much work to use and then fix all manually really sucks.

I'm glad I could help!