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.

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

options is optional in WebAuth.prototype.authorize #789

Merged
merged 3 commits into from
Jul 10, 2018
Merged

options is optional in WebAuth.prototype.authorize #789

merged 3 commits into from
Jul 10, 2018

Conversation

behrangsa
Copy link
Contributor

In your tutorials, authorize is called without any arguments (example).

Consequently, options should be marked as optional in the JsDoc (i.e. [options]), otherwise IDEs might warn that its invocations require 1 argument:

screenshot from 2018-07-06 23-03-25

In your tutorials, `authorize` is called without any arguments ([example](https://auth0.com/docs/quickstart/spa/jquery/01-login#create-an-authentication-service)).

Consequently, `options` should be marked as optional in the JsDoc (i.e. `[options]`), otherwise IDEs might warn that its invocations require 1 argument.
@behrangsa
Copy link
Contributor Author

Looks like there are quite a few functions like this in the code base:

  • WebAuth.prototype.parseHash = function(options, cb) {
  • WebAuth.prototype.renewAuth = function(options, cb) {

Might be better to reject this PR and fix all of the JsDocs in a single commit.

@luisrudge
Copy link
Contributor

You can push a new commit fixing this in other places, then we rename the PR accordingly. What do you think?

@behrangsa
Copy link
Contributor Author

@luisrudge I updated the PR with the other occurrences I could find.

@luisrudge
Copy link
Contributor

Thanks!

@luisrudge luisrudge merged commit ee463df into auth0:master Jul 10, 2018
@luisrudge luisrudge added this to the v9.7.0 milestone Jul 12, 2018
bors bot referenced this pull request in mozilla/delivery-console Jul 24, 2018
307: Update dependency auth0-js to v9.7.3 r=rehandalal a=renovate[bot]

This Pull Request updates dependency [auth0-js](https://github.com/auth0/auth0.js) from `v9.6.1` to `v9.7.3`



<details>
<summary>Release Notes</summary>

### [`v9.7.3`](https://github.com/auth0/auth0.js/blob/master/CHANGELOG.md#v973httpsgithubcomauth0auth0jstreev973-2018-07-23)
[Compare Source](auth0/auth0.js@v9.7.2...v9.7.3)
[Full Changelog](auth0/auth0.js@v9.7.3-beta1...v9.7.3)

**Fixed**
- Fix npm module export [\#&#8203;808](`https://github.com/auth0/auth0.js/pull/808`) ([luisrudge])

---

### [`v9.7.2`](https://github.com/auth0/auth0.js/blob/master/CHANGELOG.md#v972httpsgithubcomauth0auth0jstreev972-2018-07-13)
[Compare Source](auth0/auth0.js@v9.7.1...v9.7.2)
[Full Changelog](auth0/auth0.js@v9.7.1...v9.7.2)

**Fixed**
- Fix default export for auth0js [\#&#8203;803](`https://github.com/auth0/auth0.js/pull/803`) ([luisrudge])

---

### [`v9.7.1`](https://github.com/auth0/auth0.js/blob/master/CHANGELOG.md#v971httpsgithubcomauth0auth0jstreev971-2018-07-13)
[Compare Source](auth0/auth0.js@v9.7.0...v9.7.1)
[Full Changelog](auth0/auth0.js@v9.7.0...v9.7.1)

**Fixed**
- Fix build folder not being published in the tag [\#&#8203;801](`https://github.com/auth0/auth0.js/pull/801`) ([luisrudge])

---

### [`v9.7.0`](https://github.com/auth0/auth0.js/blob/master/CHANGELOG.md#v970httpsgithubcomauth0auth0jstreev970-2018-07-12)
[Compare Source](auth0/auth0.js@v9.6.1...v9.7.0)
[Full Changelog](auth0/auth0.js@v9.6.1...v9.7.0)

**Added**
- Add SRI hashes to the cdn [\#&#8203;782](`https://github.com/auth0/auth0.js/pull/782`) ([luisrudge])

**Fixed**
- options is optional in WebAuth.prototype.authorize [\#&#8203;789](`https://github.com/auth0/auth0.js/pull/789`) ([behrangsa])
- Removing `domain` option from methods (it can't be overridden) [\#&#8203;781](`https://github.com/auth0/auth0.js/pull/781`) ([luisrudge])

---

</details>




---

This PR has been generated by [Renovate Bot](https://renovatebot.com).

Co-authored-by: Renovate Bot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants