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.

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

[PHP] remove deprecated options #2083

Merged
merged 4 commits into from Feb 10, 2019

Conversation

Projects
None yet
3 participants
@wing328
Copy link
Member

wing328 commented Feb 7, 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, ./bin/security/{LANG}-petstore.sh and ./bin/openapi3/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: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

[PHP] remove deprecated options: composerVendorName, composerProjectName

cc @jebentier (2017/07), @dkarlovi (2017/07), @mandrean (2017/08), @jfastnacht (2017/09), @ackintosh (2017/09), @ybelenko (2018/07), @renepardon (2018/12)

@wing328 wing328 added this to the 4.0.0 milestone Feb 7, 2019

@ybelenko
Copy link
Contributor

ybelenko left a comment

Maybe we also can remove:

// composerVendorName/composerProjectName has be replaced by gitUserId/gitRepoId. prepare to remove these.
// public static final String COMPOSER_VENDOR_NAME = "composerVendorName";
// public static final String COMPOSER_PROJECT_NAME = "composerProjectName";
// protected String composerVendorName = null;
// protected String composerProjectName = null;

// cliOptions.add(new CliOption(COMPOSER_VENDOR_NAME, "The vendor name used in the composer package name. The template uses {{composerVendorName}}/{{composerProjectName}} for the composer package name. e.g. yaypets. IMPORTANT NOTE (2016/03): composerVendorName will be deprecated and replaced by gitUserId in the next openapi-generator release"));

// cliOptions.add(new CliOption(COMPOSER_PROJECT_NAME, "The project name used in the composer package name. The template uses {{composerVendorName}}/{{composerProjectName}} for the composer package name. e.g. petstore-client. IMPORTANT NOTE (2016/03): composerProjectName will be deprecated and replaced by gitRepoId in the next openapi-generator release"));

// if (additionalProperties.containsKey(COMPOSER_PROJECT_NAME)) {
// this.setComposerProjectName((String) additionalProperties.get(COMPOSER_PROJECT_NAME));
// } else {
// additionalProperties.put(COMPOSER_PROJECT_NAME, composerProjectName);
// }

// if (additionalProperties.containsKey(COMPOSER_VENDOR_NAME)) {
// this.setComposerVendorName((String) additionalProperties.get(COMPOSER_VENDOR_NAME));
// } else {
// additionalProperties.put(COMPOSER_VENDOR_NAME, composerVendorName);
// }

// public void setComposerVendorName(String composerVendorName) {
// this.composerVendorName = composerVendorName;
// }
// public void setComposerProjectName(String composerProjectName) {
// this.composerProjectName = composerProjectName;
// }

@wing328

This comment has been minimized.

Copy link
Member Author

wing328 commented Feb 8, 2019

Good catch. I've pushed a comment to remove not only those commented codes in the abstract php class but also the templates (lumen, symfony, etc)

Fixed via 1c04082

@ackintosh
Copy link
Member

ackintosh left a comment

LGTM 👍

@ackintosh ackintosh merged commit 20d5adc into master Feb 10, 2019

6 checks passed

Shippable Run 5753 status is SUCCESS.
Details
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@ackintosh ackintosh deleted the php-remove-options branch Feb 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment