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

NelmioApiDoc parser location #156

Closed
mRoca opened this issue Jul 7, 2015 · 5 comments
Closed

NelmioApiDoc parser location #156

mRoca opened this issue Jul 7, 2015 · 5 comments

Comments

@mRoca
Copy link
Contributor

mRoca commented Jul 7, 2015

Hello, I have a simple question: why put the Nelmio documentation extractors in the NelmioApiDocBundle ? I don't think that the DunglasApiBundle specific PR have a place in the source code of the Nelmio project, what do you think of it ?

For example, I want to create this PR : nelmio/NelmioApiDocBundle@master...mRoca:feature/dunglas-avoid-infinite-recursion , and I would like to add some tests to prevent infinite recursion with circular entities relations with the same serializer groups. As this items are DunglasApiBundle specific, add them in the NelmioApiDocBundle doesn't seem normal.

Other example with this potential PR (wip) : nelmio/NelmioApiDocBundle@master...mRoca:feature/dunglas-add-max-depth . In order to add a default MAX_DEPTH option in the Nelmio doc, I would like to add a config option, or something like it.

@ogizanagi
Copy link
Contributor

I also wondered this a while ago, and there is only something which could be problematic, I think:
How will the DunglasApiBundle tests the NelmioApiDocBundle behavior if tests should also belong to this bundle ?

This means the DunglasApiBundle should have a dependency on nelmio/api-doc-bundle for tests (a script could be execute first in order to add the dependency) and replicate a big part of what is done in the bundle ?

@dunglas
Copy link
Member

dunglas commented Jul 7, 2015

@ogizanagi yes it's exactly why I've opened the PR on NelmioApiDoc initially. On another hand, I've no problem having NelmioApiDoc in the require-dev section of this bundle.

@ogizanagi
Copy link
Contributor

Then we have our answer 😄

How would you proceed if some features targeting the nelmio api doc behavior requires some configuration ?
Will you put this config in the DunglasApiBundle ?

@ogizanagi
Copy link
Contributor

I feel like the fact Nelmio related stuff is located in another bundle is quite a big issue, especially until the api bundle has its first stable release (For instance, the dev-master is now broken due to the move of metadata factories interfaces). I don't think we should have another BC break factor.

I think the nelmio related stuff should move back into the bundle. It could be required in dev only, or added by Travis on build. Tests could also be marked by a dedicated group in order to exclude them when executing tests locally.

@teohhanhui
Copy link
Contributor

Fixed by #410

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

No branches or pull requests

4 participants