Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Use the latest version of superagent. #3579

Merged
merged 2 commits into from
Aug 13, 2019
Merged

Use the latest version of superagent. #3579

merged 2 commits into from
Aug 13, 2019

Conversation

oponder
Copy link
Contributor

@oponder oponder commented Aug 7, 2019

Intro

Hi! Thanks for this great project :D

This is a small PR to upgrade superagent in the generated javascript clients.

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

Upgrade superagent to version 5.1.0, since versions lower than 3.8.1 have a security vulnerability.
https://app.snyk.io/vuln/npm:superagent:20181108

We had been getting around this by manually editing the generated package.json each time. 5.1.0 seems like a big leap, but there appear to be no relevant breaking changes, at least not for our client: https://github.com/visionmedia/superagent#upgrading-from-previous-versions

So I believe this could be filed against master, but please correct me if I'm wrong!

Of course, I lack the full overview on what might break, so requesting a second pair of eyes, and confirmation here.

cc: @jfiala, @achew22, @jaypea

Regarding "Ran the shell script under ./bin/" from the PR checklist, I think I ran ./bin/javascript-petstore-all, ./bin/openapi3/javascript-es6-petstore.sh and ./bin/openapi3/javascript-closure-angular.sh

Does that cover it all? Wasn't sure about the right thing to do here.

Quite some more changes than expected came in though in the examples.

@wing328
Copy link
Member

wing328 commented Aug 13, 2019

@oponder thanks for the PR. Are you free for a quick chat on this?

Please PM me via Slack

Copy link
Member

@wing328 wing328 left a comment

Choose a reason for hiding this comment

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

LGTM

@wing328 wing328 merged commit 5e27f11 into OpenAPITools:master Aug 13, 2019
@wing328
Copy link
Member

wing328 commented Aug 26, 2019

@oponder thanks for the PR, which has been included in the v4.1.1 release: https://twitter.com/oas_generator/status/1165944867391860737

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants