Skip to content

Conversation

@7474
Copy link
Contributor

@7474 7474 commented Dec 9, 2018

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Proposed changes

swagger.security is a definition for the whole API. It is not necessarily set when there is security on individual methods.

Therefore it is better to refer to swagger.securityDefinitions to see if you need hasServiceParams.

isSecure judgment also refers to swagger.securityDefinitions.

Ref: https://github.com/wcandillon/swagger-js-codegen/blob/master/lib/codegen.js#L41

Checklist

  • I have read the contribution guidelines
  • For non-bugfix PRs, I have discussed this change on the mailing list/slack team.
  • I have run grunt to verify the unit tests pass
  • I have added suitable unit tests to cover the new/changed functionality

`swagger.security` is a definition for the whole API. It is not necessarily set when there is security on individual methods.

Therefore it is better to refer to `swagger.securityDefinitions` to see if you need ` hasServiceParams`.

`isSecure` judgment also refers to ` swagger.securityDefinitions`.

Ref: https://github.com/wcandillon/swagger-js-codegen/blob/master/lib/codegen.js#L41
@jsf-clabot
Copy link

jsf-clabot commented Dec 9, 2018

CLA assistant check
All committers have signed the CLA.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 65.928% when pulling d4e8b8c on 7474:patch-1 into b39ecda on node-red:master.

@kazuhitoyokoi kazuhitoyokoi merged commit 744eab1 into node-red:master Feb 27, 2019
@kazuhitoyokoi
Copy link
Member

@7474 Sorry for the delay. 🙏 Thank you for your contribution. 👍

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants