Skip to content

Conversation

@ePaul
Copy link
Contributor

@ePaul ePaul commented Mar 10, 2017

PR checklist

  • Read the contribution guildelines.
  • Ran the shell/batch script under ./bin/ to update Petstore sample so that CIs can verify the change.
    → ran bin/dart-petstore.sh, with no changes in the produced samples.
  • Filed the PR against the correct branch: master for non-breaking changes

Description of the PR

This fixes (or hopes to fix) part of #4898 for the dart generator.

The problem likely occurred when a path parameter name (as declared in the OpenAPI definition) is of a form which doesn't match the form used in Dart, or is a reserved word there. In this case the parameter gets renamed for use in the code, and the algorithm for creating the actual path tries to replace the renamed parameter in the path (which will do nothing), not the original one (which would be correct).

This commit changes the replacement to using {{baseName}} (which is the original name of the parameter definition as parsed from the API definition) instead of {{paramName}} (which is the sanitized version of the parameter name).

Our Petstore API definition doesn't seem to contain a path parameter which would get modified by the sanitation, therefore there is no difference in the generated samples.

I don't know anything about Dart, so this needs review by Dart experts. /cc @yissachar @close2 @ircecho @hcwilhelm

@wing328
Copy link
Contributor

wing328 commented Mar 12, 2017

@ePaul thanks for the fix, which looks good to me (although I'm no expert in Dart)

@wing328 wing328 merged commit c76f006 into swagger-api:master Mar 12, 2017
@ePaul ePaul deleted the fix-partially-4898-for-dart branch March 12, 2017 10:29
@wing328 wing328 changed the title fix (partially) #4898 for dart. [Dart] Fix parameter names with {{baseName}} Mar 12, 2017
spr3nk3ls pushed a commit to spr3nk3ls/swagger-codegen that referenced this pull request Mar 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants