Improve Swagger's DocumentationNormalizer #740
Conversation
dunglas
commented
Sep 8, 2016
dunglas
commented
| Q | A |
|---|---|
| Bug fix? | no |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | n/a |
| License | MIT |
| Doc PR | n/a |
| ], | ||
| '404' => [ | ||
| 'description' => 'Resource not found', | ||
| $pathOperation['tags'] ?? $pathOperation['tags'] = [$resourceShortName]; |
There was a problem hiding this comment.
Git(Hub) fails to create a good diff, but the underlying code looks ok.
There was a problem hiding this comment.
I mean, shouldn't it be:
$pathOperation['tags'] = $pathOperation['tags'] ?? [$resourceShortName];There was a problem hiding this comment.
Why? It makes an useless assignation.
There was a problem hiding this comment.
IMO that's a bad coding style. Too cryptic and most likely doesn't even have any practical benefit.
It's short-circuit on steroids... lol
There was a problem hiding this comment.
I find it clearer but it's probably a personal taste :)
There was a problem hiding this comment.
It's called job-safety not short-circuit on steroids 😸
| @@ -129,8 +133,8 @@ private function parseResource(ResourceMetadata $resourceMetadata, string $resou | |||
| private function getGroupsForItemAndCollectionOperation(ResourceMetadata $resourceMetadata, string $operationName) : array | |||
| { | |||
| $operation = [ | |||
| 'denormalization_context' => array_merge($resourceMetadata->getItemOperationAttribute($operationName, 'denormalization_context', []), $resourceMetadata->getCollectionOperationAttribute($operationName, 'denormalization_context', [])), | |||
| 'normalization_context' => array_merge($resourceMetadata->getItemOperationAttribute($operationName, 'normalization_context', []), $resourceMetadata->getCollectionOperationAttribute($operationName, 'normalization_context', [])), | |||
| 'denormalization_context' => $resourceMetadata->getItemOperationAttribute($operationName, 'denormalization_context', []) + $resourceMetadata->getCollectionOperationAttribute($operationName, 'denormalization_context', []), | |||
There was a problem hiding this comment.
Why is there union of item + collection operation attributes going on? Only one of them can be applicable at a time.
Even if the intention is to only get one of them, what about collection and item operations with the same name? Too risky...
There was a problem hiding this comment.
I 100% agree, this must be investigated. But this PR doesn't change the behavior, it's just a style fix. I propose to refactor this method in another PR. WDYT?
Improve Swagger's DocumentationNormalizer
