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.

Sign up for GitHub

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

Jump to bottom

Typescript+Axios: Separate model and api classfiles and package #2005

Merged
merged 23 commits into from
Mar 5, 2019
Merged

Typescript+Axios: Separate model and api classfiles and package #2005

merged 23 commits into from
Mar 5, 2019

Conversation

mvniekerk
Copy link
Contributor

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 and ./bin/security/{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\.
  • Filed the PR against the correct branch: master, 3.4.x, 4.0.x. Default: 4.0.x.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

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

Description of the PR

This PR needs to follow/branches #2002

What it does:

  • Optionally splits API and Model class files for Axios in the apiPackage and modelPackage directory
  • Filenames are kebab case (MyAwesomeClass -> my-awesome-class.ts)

@mvanniekerkdvt
Typescript 3.2
62e46f6
@mvanniekerkdvt
Typescript spread operator
6064f57
@mvanniekerkdvt
Add vendor extension to the operation
c5ef723
@mvanniekerkdvt
Remove url.URLSearchParams
8a32553
@mvanniekerkdvt
Generate form data in API
dce5438
@mvanniekerkdvt
Make axios scripts executable
b45432b
@mvanniekerkdvt
Reran generator
6452cfe
@mvanniekerkdvt
Generate sample code
b661748
@mvanniekerkdvt
Codegen having the model and api extra flag
d952ffd
@mvanniekerkdvt
Revert to 2.4 Typescript
9818ee8
@mvanniekerkdvt
COLLECTION_FORMAT.{{collectionFormat}} everywhere for consistency
9e33519
@mvanniekerkdvt
Consistency on the CollectionFormats, comment on the vendor extension
3ba25dc
@mvanniekerkdvt
Merge branch 'bugfix-multipart-form-data' into feature-separate-model…
e771a74 
…-and-api
@mvanniekerkdvt
Throw exception if api and model packages are not given
b90daa0
@mvanniekerkdvt
Templates splitting api and models
c948769
@mvanniekerkdvt
Post process the operations in the the process
c0f23fe
@mvanniekerkdvt
Beginning to get the axios tests up
7a75648
@mvanniekerkdvt
Ensure-up-to-date doesnt include TS/Axios tests
fa79b64
@mvanniekerkdvt
Docs update for Typescript/Axios
fd5f7dc
@mvniekerk mvniekerk mentioned this pull request Jan 28, 2019
Closed
6 tasks
@macjohnny
Copy link
Member

please request review once #2002 is merged

@mvniekerk
Copy link
Contributor Author

please request review once #2002 is merged

Will do

@wing328 wing328 changed the base branch from 4.0.x to master February 10, 2019 07:46
@wing328
Copy link
Member

wing328 commented Feb 13, 2019

#2002 has been merged into master.

@mvniekerk can you resolve the merge conflicts when you've time? let me know if you need any help on that.

@wing328 wing328 added this to the 4.0.0 milestone Feb 13, 2019
@mvanniekerkdvt
Merge branch 'master' into feature-separate-model-and-api
c099c26 
# Conflicts:
#	docs/generators/typescript-axios.md
#	modules/openapi-generator/src/main/resources/typescript-axios/api.mustache
#	samples/client/petstore/typescript-axios/builds/default/api.ts
#	samples/client/petstore/typescript-axios/builds/es6-target/api.ts
#	samples/client/petstore/typescript-axios/builds/with-interfaces/api.ts
#	samples/client/petstore/typescript-axios/builds/with-npm-version/api.ts
@mvanniekerkdvt
Merge master
ed31f45
@mvanniekerkdvt
Api tests re-run
d873641
@mvniekerk
Copy link
Contributor Author

@wing328 @macjohnny - made it mergable again.

@mvniekerk
Copy link
Contributor Author

Not to be pushy -
Kindly accept or reject this PR, please. I'll be leaving Grindrod Bank end of March and my access to their forked repository is not a given post March 2019. Would like to wrap up and clean up.
CC @macjohnny @wing328

macjohnny
macjohnny approved these changes Mar 4, 2019
View reviewed changes
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.

IMHO it looks good. There are only minimal, non-breaking changes in the existing samples. I haven't tested the new samples with separated model and api, but at first glance this looks good, too.
@mvniekerk can you confirm that you tested the newly generated files in an application?

@macjohnny
Copy link
Member

@mvniekerk thanks a lot for your effort!

@macjohnny
Copy link
Member

@nicokoenig could you please have a quick look at this one?

@mvniekerk
Copy link
Contributor Author

@mac

IMHO it looks good. There are only minimal, non-breaking changes in the existing samples. I haven't tested the new samples with separated model and api, but at first glance this looks good, too.
@mvniekerk can you confirm that you tested the newly generated files in an application?

Hey @macjohnny
Yes - running in production code

@macjohnny
Copy link
Member

@wing328 I think this should be good to go

@wing328 wing328 merged commit caf404d into OpenAPITools:master Mar 5, 2019
@mvniekerk mvniekerk deleted the feature-separate-model-and-api branch March 5, 2019 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@macjohnny macjohnny macjohnny approved these changes

Assignees
No one assigned
Labels
Client: TypeScript Enhancement: General
Projects
None yet
Milestone
4.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@mvniekerk @macjohnny @wing328 @mvanniekerkdvt