maxdome / swagger-combine

Combines multiple Swagger schemas into one dereferenced schema.
MIT License
132 stars 36 forks source link

Add BasePath Support for #81 #82

Closed dawinter closed 5 years ago

dawinter commented 5 years ago

https://github.com/maxdome/swagger-combine/issues/81

dawinter commented 5 years ago

@fabsrc May want to look into?

fabsrc commented 5 years ago

Thanks for your PR @dawinter

The functionality makes sense, however this seems like a breaking change to me, because it changes the default logic which might break existing services that are using this module.

Could you maybe add an option to activate this functionality (similar to existing options like includeParameters)?

Also tests would be great.

dawinter commented 5 years ago

Thanks for the first review. First I thought it must be a bug. But you are right the testcase shows that the error behavoir was expected. I added a parameter and a test case.

Please review again. Feel free to change the parameter name or the level of the parameter.

fabsrc commented 5 years ago

Looks good to me. 🙂 Could you please also add some documentation for this feature?

dawinter commented 5 years ago

documentation added ad base path option moved to the paths section to respect the structure of options.