search

Azure / openapi-diff

Command line tool to detect breaking changes between two openapi specifications
MIT License
257 stars 36 forks source link

Oad reports added required parameter when it just moved from path to operation #220

Closed mikekistler closed 2 years ago

mikekistler commented 2 years ago

In this PR, oad reports many instances of "AddingRequiredParameter" that are false positives, because the parameter was present and required in the previous version, but was specified at the path level rather than in the operation, as it is in the new version.

While this is a change in the way the API is described, it is not actually a change in the API definition, so should not be reported as a breaking change.

jianyexi commented 2 years ago

This is not false alarm, removing the path level parameters is also breaking change

mikekistler commented 2 years ago

In this case, path parameters were neither added nor removed. They were simply described in a different place.

mikekistler commented 2 years ago

Not sure what is going on -- I've tried to get a simple recreate for this problem but so far I am not successful.

mikekistler commented 2 years ago

I think I may see what is going on. The PR was updated but the PR checks were not marked as "outdated" or updated in place. This makes it look like the current code has problems when in fact the problems were fixed.

If this is correct then we can close this issue but should open one on the pipeline to update the PR checks section in place.