Skip to content

Conversation

@ePaul
Copy link
Contributor

@ePaul ePaul commented Mar 4, 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.
    → bin/swift3-petstore-all.sh (once before the change, see Update swift3 samples after 2.2.2. #4911, once again after the change.)
  • Filed the PR against the correct branch: master for non-breaking changes.

Description of the PR

This includes the changes of #4911 – after merging those, it should be clearer what actually changed. (Or just have a look at the last two commits.)

This fixes a wrong name-mapping of path parameters, as part of solving #4898.

It looks like there is no test case in petstore which actually makes a difference here.
It might be that there actually is no problem (i.e. if parameter names are never modified for Swift3, then baseName is the same as paramName, so it doesn't matter – though then there might be problems when hitting reserved words), or that it just is not happens with our petstore example.

I need someone who knows about the swift clients to review this – cc @jaz-ah @Edubits.

(A side remark: Some ids in the generated samples look different each time the generator is run – see commit 8e847ab. This introduces unneeded noise when looking at the diffs. Might be worth looking into, in a separate issue/PR.)

@ePaul ePaul changed the title Fix 4898 for swift3 Fix #4898 for swift3 Mar 4, 2017
@wing328
Copy link
Contributor

wing328 commented Mar 5, 2017

@ePaul thanks for the fix.

cc @hexelon @jaz-ah @Edubits @jgavris

@wing328 wing328 added this to the v2.2.3 milestone Mar 5, 2017
@ePaul ePaul force-pushed the fix-4898-for-swift3 branch from 8e847ab to 101cf8d Compare March 6, 2017 10:05
@ePaul
Copy link
Contributor Author

ePaul commented Mar 6, 2017

Rebased onto master (and reran the samples generation) to get rid of conflicts.

@ePaul ePaul force-pushed the fix-4898-for-swift3 branch from 101cf8d to 0618885 Compare March 6, 2017 14:49
@ePaul ePaul changed the title Fix #4898 for swift3 Fix (partially) #4898 for swift3 Mar 6, 2017
@wing328
Copy link
Contributor

wing328 commented Mar 7, 2017

LGTM. Thanks for the fix.

@wing328 wing328 merged commit 404a999 into swagger-api:master Mar 7, 2017
@ePaul ePaul deleted the fix-4898-for-swift3 branch March 7, 2017 14:02
@wing328 wing328 changed the title Fix (partially) #4898 for swift3 [Swift3] Fix parameter names using {{baseName}} Mar 12, 2017
spr3nk3ls pushed a commit to spr3nk3ls/swagger-codegen that referenced this pull request Mar 28, 2017
* fix (partially) swagger-api#4898 for swift3.

* Update petstore samples for swift3 (after fix for swagger-api#4898)
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