Make source folder configurable #949
Conversation
| @@ -62,6 +62,7 @@ protected function configureContainer(ContainerBuilder $c, LoaderInterface $load | |||
|
|
|||
| // Filter routes | |||
| $c->loadFromExtension('nelmio_api_doc', [ | |||
| 'source_folder' => '%kernel.root_dir%', | |||
There was a problem hiding this comment.
%kernel.root_dir%/../src?
There was a problem hiding this comment.
I'd like the bundle to work without config in a standard symfony app.
There was a problem hiding this comment.
The default value is indeed %kernel.root_dir%/../src (set in Configuration), but the testing app/kernel in src/Test/Functional does not have a src folder. Should I create one?
There was a problem hiding this comment.
Indeed reviewed too quickly :)
|
Great, thanks ! |
| @@ -22,6 +22,7 @@ public function getConfigTreeBuilder() | |||
| $treeBuilder | |||
| ->root('nelmio_api_doc') | |||
| ->children() | |||
There was a problem hiding this comment.
BTW i wonder, shouldn't we always require swagger-php? Or otherwise maybe it is better to put this in a sub section swagger_php
There was a problem hiding this comment.
I was under the impression that the source_folder option might be used for multiple bundle, but I might be wrong.
There was a problem hiding this comment.
I don't see other use cases.
In fact we could maybe even get ride of this option if we use our own parser with Swagger-Php annotations that would focus on controllers but that would mean losing support of Swagger-Php annotations for entities and only support @Model. Wdyt @dbu?
There was a problem hiding this comment.
if we can use what swagger-php does, we should hard require that imho.
whether to use our own parser or not, i don't know. i was really surprised to see swagger-php having its own php parser code instead of using something like the doctrine annotations tool to do that... not a fan of "sharing" annotations with swagger-php but parsing them differently. maybe its easier to fork swagger-php and make the library clean and extensible to work well together with the bundle? and then maybe the swagger-php people are ok to take that library as a next major version so it does not have to be a permanent fork...
There was a problem hiding this comment.
if we can use what swagger-php does, we should hard require that imho.
Will do.
i was really surprised to see swagger-php having its own php parser code instead of using something like the doctrine annotations tool to do that...
Actually they use the doctrine parser to parse docblocks but they don't want to execute the code (which is forced by reflection) so they added a layer on top of it to make it static.
maybe its easier to fork swagger-php and make the library clean and extensible to work well together with the bundle?
I don't think it's worth it, I prefer merging our efforts :)
make the library clean and extensible to work well together with the bundle
It would be great but I don't think it's doable without loosing its flexibility to work with any project.
There was a problem hiding this comment.
did not want to promote a split. all the better if you can get it working with swagger-php ;-)
|
What do you think of this workaround? |
Fix for #947
The fact that the test are still passing with this change are the only test included :D
Maybe I should start testing the Extension?