Skip to content

Conversation

agologan
Copy link
Contributor

Pretty self explanatory, helps with calling APIs with many parameters as you don't have to specify all params.

Considered also initialising with default value but that's more like the value the server assigns when a param is missing so I don't think it's wise to pre-initialize it in the client.

@codecov
Copy link

codecov bot commented Nov 18, 2020

Codecov Report

Merging #145 (6454f3b) into master (54394ee) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #145   +/-   ##
=========================================
  Coverage     71.95%   71.95%           
  Complexity      139      139           
=========================================
  Files            11       11           
  Lines           574      574           
  Branches         75       75           
=========================================
  Hits            413      413           
  Misses          118      118           
  Partials         43       43           

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 54394ee...a7ef162. Read the comment docs.

@cortinico cortinico self-requested a review November 18, 2020 19:50
@cortinico cortinico added this to the 2.0.0 milestone Nov 18, 2020
@cortinico cortinico added the enhancement New feature or request label Nov 18, 2020
Copy link
Collaborator

@cortinico cortinico left a comment

Choose a reason for hiding this comment

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

Change looks fine to me 👍 I'd love to get an ack from @macisamuele as well.
Moreover, @agologan can I ask you to add a test here https://github.com/Yelp/swagger-gradle-codegen/blob/master/samples/junit-tests/src/test/java/com/yelp/codegen/generatecodesamples/ValidParameterTest.kt for a call to getSymbolsInParameterName() with no parameters?

@agologan
Copy link
Contributor Author

agologan commented Nov 19, 2020

[...] can I ask you to add a test [...] for a call to getSymbolsInParameterName() with no parameters?

done.

Copy link
Collaborator

@macisamuele macisamuele left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Thanks a lot for contributing.

@cortinico will you follow-up with merging (and eventually releasing) this?

@cortinico
Copy link
Collaborator

@cortinico will you follow-up with merging (and eventually releasing) this?

Let's merge this.
For the releasing part, I'd love to have this into 2.x
I'll have some time during the XMas eve to review/develop all the remaining features for 2.x

@cortinico cortinico merged commit 8b24632 into Yelp:master Nov 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants