Skip to content

Comments

[typescript-angular] Add encoder configuration, fix default encoder#3389

Merged
macjohnny merged 7 commits intoOpenAPITools:masterfrom
cubidobusinesssolutions:master
Jul 22, 2019
Merged

[typescript-angular] Add encoder configuration, fix default encoder#3389
macjohnny merged 7 commits intoOpenAPITools:masterfrom
cubidobusinesssolutions:master

Conversation

@cubidobusinesssolutions
Copy link
Contributor

@cubidobusinesssolutions cubidobusinesssolutions commented Jul 18, 2019

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\. If contributing template-only or documentation-only changes which will change sample output, be sure to build the project first.
  • Filed the PR against the correct branch: master, 4.1.x, 5.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

(details of the change, additional tests that have been done, reference to the issue for tracking, etc)

fixes #3372

#3372)

* Add encoder to configuration
* Fix import indention
* Default encoder workaround for angular/angular#18261
@auto-labeler
Copy link

auto-labeler bot commented Jul 18, 2019

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

@macjohnny
Copy link
Member

cc @TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09) @macjohnny (2018/01) @nicokoenig (2018/09) @topce (2018/10) @akehir (2019/07)

Copy link
Member

@macjohnny macjohnny left a comment

Choose a reason for hiding this comment

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

Thanks for your PR! I think this is a very helpful configuration option.
I suggest to move the decision logic to the constructor.

* Extract encoder class variable initialized in constructor
Copy link
Member

@macjohnny macjohnny left a comment

Choose a reason for hiding this comment

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

LGTM

@macjohnny
Copy link
Member

@cubidobusinesssolutions can you please re-generate the samples?

Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows. If contributing template-only or documentation-only changes which will change sample output, be sure to build the project first.

@akehir
Copy link
Contributor

akehir commented Jul 20, 2019

Very nice, thanks !

@cubidobusinesssolutions
Copy link
Contributor Author

cubidobusinesssolutions commented Jul 22, 2019

@macjohnny

can you please re-generate the samples?

I was hoping I would not have to install Java JRE, Java SDK, and Maven. See additional commits.

@macjohnny
Copy link
Member

the remaining CI failure is unrelated

@macjohnny macjohnny merged commit f436904 into OpenAPITools:master Jul 22, 2019
@wing328 wing328 added this to the 4.1.0 milestone Aug 9, 2019
@wing328 wing328 changed the title [typescript-angular] Add encoder configuration, fix default encoder (#3372) [typescript-angular] Add encoder configuration, fix default encoder Aug 9, 2019
@wing328
Copy link
Member

wing328 commented Aug 10, 2019

@cubidobusinesssolutions thanks for the PR, which has been included in the 4.1.0 release: https://twitter.com/oas_generator/status/1160000504455319553

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.

[BUG] TypeScript Angular Escape colon

5 participants