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

#80: ensure typescript samples are up to date #444

Merged
merged 13 commits into from
Jul 6, 2018
Merged

#80: ensure typescript samples are up to date #444

merged 13 commits into from
Jul 6, 2018

Conversation

macjohnny
Copy link
Member

@macjohnny macjohnny commented Jul 3, 2018 •
edited
Loading

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.1.x, 4.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

ensure typescript samples are up to date, see #80 (comment)

cc @jmini

@macjohnny
Copy link
Member Author

@jmini are there any plans to ignore changes to the VERSION file?

@jmini
Copy link
Member

jmini commented Jul 3, 2018

@macjohnny:
In our samples/ folder? this was discussed in #245. My opinion is that this is useful information (feel free to continue the discussion there - reopen if necessary).
If you do not want to generate it for your project, I think that there is an option to not generate metadata files.

@macjohnny
Copy link
Member Author

@jmini

In our samples/ folder?

yes, I think it is cumbersome needing to re-generate all samples when the version number is updated

@jmini
Copy link
Member

jmini commented Jul 3, 2018 •
edited
Loading

yes, I think it is cumbersome needing to re-generate all samples when the version number is updated

This is made as part of the "Prepare next version PR" (see 9511586 for an example), you do not have to care about this.

Having the .openapi-generator/VERSION file in samples has several benefits:

  • understanding the state of a samples. We used it a lot during migration to 3.0.0.
  • seeing outdated samples, sorting samples by versions

The .openapi-generator/VERSION is not the only file. Some generator also output the OpenAPI-Generator version in their header comment.

See also: https://github.com/OpenAPITools/openapi-generator/wiki/Git-Branches#solving-conflicts-during-merge

@macjohnny
Copy link
Member Author

@jmini ok, so then it should be fine.

@jmini
Copy link
Member

jmini commented Jul 3, 2018

After merge of #363 the merge conflicts needs to be solved, sorry about this (this will happen less and less when the samples stays up-to-date with each PR)

@macjohnny
Copy link
Member Author

@jmini no problem, I will update the samples tomorrow and try to fix the typescript-node test that fails with the updated samples

@macjohnny macjohnny closed this Jul 5, 2018
@macjohnny macjohnny reopened this Jul 5, 2018
@macjohnny
Copy link
Member Author

macjohnny commented Jul 5, 2018 •
edited
Loading

@jmini @wing328 could you please merge this? all checks pass, except for (unrelated) outdated samples:

  • samples/client/petstore/java/webclient/docs/FakeApi.md
  • samples/client/petstore/java/webclient/src/main/java/org/openapitools/client/api/FakeApi.java
  • samples/openapi3/client/petstore/php/OpenAPIClient-php/lib/Api/PetApi.php
  • samples/client/petstore/java/webclient/docs/FileSchemaTestClass.md (new file)
  • samples/client/petstore/java/webclient/src/main/java/org/openapitools/client/model/FileSchemaTestClass.java (new file)

would you mind updating these files in a separate PR?

@jmini
Copy link
Member

jmini commented Jul 6, 2018 •
edited
Loading

would you mind updating these files in a separate PR?

I think @wing328 just did that in #478

@jmini
Copy link
Member

jmini commented Jul 6, 2018

I have merged master into your PR.

jmini
jmini approved these changes Jul 6, 2018
View reviewed changes
Copy link
Member

@jmini jmini left a comment

Choose a reason for hiding this comment

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

If CI jobs are OK, then the PR can be merged

@macjohnny
Copy link
Member Author

@jmini @wing328 thanks! The CI checks now pass.

@jmini jmini merged commit 3408866 into OpenAPITools:master Jul 6, 2018
@jmini jmini added this to the 3.1.0 milestone Jul 6, 2018
jimschubert added a commit to jimschubert/openapi-generator that referenced this pull request Jul 7, 2018
@jimschubert
Merge branch 'master' into generator-validation-option
232166e 
* master: (116 commits)
  3.1.0 release (OpenAPITools#486)
  Fix broken link to openapi generator plugin README (OpenAPITools#484)
  show warning message for nodejs server only (OpenAPITools#481)
  Add grokify to Go technical committee (OpenAPITools#479)
  [Slim] Improve codebase decouple (OpenAPITools#438)
  Ensure typescript samples are up to date (OpenAPITools#444)
  Update README.md
  [Golang][client] delete sample output dir before rebuild (OpenAPITools#477)
  update petstore samples (OpenAPITools#478)
  Revert "Improve Docker Tags (OpenAPITools#390)"
  update go client test dependencies (OpenAPITools#468)
  [Golang][client] fix for schema definition name `file` (OpenAPITools#433)
  Fix '.travis' file (syntax)
  make LICENSE GitHub display compatible (OpenAPITools#467)
  Improve Docker Tags (OpenAPITools#390)
  [Golang][client] fix file suffix for _test.go (OpenAPITools#449)
  Remove copy section (OpenAPITools#463)
  Add link to presentation (OpenAPITools#465)
  Use postProcessOperationsWithModels(Map, List) (OpenAPITools#431)
  [C] Adding petstore sample written in C (OpenAPITools#306)
  ...
@macjohnny macjohnny mentioned this pull request Oct 24, 2018
Closed
A-Joshi pushed a commit to ihsmarkitoss/openapi-generator that referenced this pull request Feb 27, 2019
@macjohnny @jmini
Ensure typescript samples are up to date (OpenAPITools#444)
c53724e 
See OpenAPITools#80
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wing328 wing328 wing328 approved these changes

@jmini jmini jmini approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.1.0
Development

Successfully merging this pull request may close these issues.
3 participants
@macjohnny @jmini @wing328