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

714 query parameter collection format rule migration #805

Merged

Conversation

maxim-tschumak
Copy link
Contributor

Related to #714.
Related to the Guideline change proposed in zalando/restful-api-guidelines#440

@codecov-io
Copy link

codecov-io commented Aug 21, 2018

Codecov Report

Merging #805 into master will decrease coverage by 0.23%.
The diff coverage is 58.53%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #805      +/-   ##
============================================
- Coverage     84.61%   84.37%   -0.24%     
+ Complexity      994      992       -2     
============================================
  Files           172      171       -1     
  Lines          2743     2740       -3     
  Branches        420      421       +1     
============================================
- Hits           2321     2312       -9     
- Misses          213      215       +2     
- Partials        209      213       +4
Impacted Files Coverage Δ Complexity Δ
.../zally/rule/zalando/PluralizeNamesForArraysRule.kt 71.42% <100%> (ø) 7 <6> (ø) ⬇️
...zalando/zally/rule/zalando/FormatForNumbersRule.kt 63.63% <100%> (ø) 11 <8> (ø) ⬇️
...src/main/java/de/zalando/zally/util/OpenApiUtil.kt 48.64% <17.64%> (-26.36%) 0 <0> (ø)
...rule/zalando/QueryParameterCollectionFormatRule.kt 77.77% <75%> (-16.34%) 8 <7> (-2)
...zalando/zally/rule/zalando/CommonFieldTypesRule.kt 90.47% <92.85%> (ø) 10 <7> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 21d7e59...35ca4da. Read the comment docs.

.filter { "query" == it.value.`in` && "array" == it.value.schema.type }
.filter { it.value.style == null || allowedStyle != it.value.style }
.map { context.violation(description, it.value) }
else emptyList()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would here either use .values (since the key is not needed) or use deconstruction { (_, value) -> ...}. Both ways would avoid the .value call.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good hint!

* Returns all defined schemas of an API specification
* @return a collection of schemas
*/
fun OpenAPI.getAllSchemas(): Collection<Schema<Any>> = this.components.schemas.orEmpty().values +
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maxim-tschumak :
What happened to the long exhaustive version of that that I did some weeks ago...? Did we discarded it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm... I don't know. This is the function I've implemented last week and now I've just moved it to the right class (as you proposed).

@ghost
Copy link

ghost commented Aug 21, 2018

👍

@maxim-tschumak
Copy link
Contributor Author

👍

@maxim-tschumak maxim-tschumak merged commit 43b1890 into master Aug 22, 2018
@maxim-tschumak maxim-tschumak deleted the 714-query-parameter-collection-format-rule-migration branch August 22, 2018 07:44
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

Successfully merging this pull request may close these issues.

None yet

2 participants