search

Redocly / swagger-repo

CLI tool to help you manage your OpenAPI(fka Swagger) repo.
MIT License
18 stars 29 forks source link

Allow code samples in spec and in files? #7

Closed phillbaker closed 6 years ago

phillbaker commented 6 years ago

Hello and thanks for all the hard work on this!

I'm curious why the build validates if all the code samples are either in the swagger.yml or in separate files? What was the motivation behind adding this check (the commit which added it didn't help: https://github.com/Rebilly/swagger-repo/commit/32701eb3ab1aaf4ad0827035458d1169f80b4f52)

https://github.com/Rebilly/swagger-repo/blob/868d52c2d17c9ceab83d58b1f31e495daaf7db4a/lib/index.js#L205

Could we define a precedence, e.g. code samples defined in files override those defined in the spec file, and then just merge them?

IvanGoncharov commented 6 years ago

Hi @phillbaker,

Thanks for motivating words.

I'm curious why the build validates if all the code samples are either in the swagger.yml or in separate files?

I think it's two distinct styles (like spaces vs tabs) and it perfectly fine to use either one. But the real problems begin when you mixing ones.

In general, this tool was designed to help teams managing their Swagger specs so we included checks to ensure that all team members follow the same workflow.

Could we define a precedence, e.g. code samples defined in files override those defined in the spec file, and then just merge them?

Yes, we could. However, if you have competing code samples something went wrong and it should be fixed. But the only thing we can do is output warning message and no one reading warning messages in CI logs so this issue will go unnoticed until it will shoot you in the leg. Our current check was specifically designed to prevent such situations.

But I understand this was a pretty opinionated decision and maybe we missed some valid use cases. Would be great to hear more details about your use case?

phillbaker commented 6 years ago

Thanks @IvanGoncharov, that all makes sense and is very reasonable.

In our case, we have some examples that can be automatically generated (which we generate and update outside the spec file) while some that must be generated by hand, which we'd like to keep next to the specs in the file to catch during code review.

I think we figured out a happy medium by altering the default script/build.js, so I'll close this issue out. Thanks!