-
Notifications
You must be signed in to change notification settings - Fork 7
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
style: Manual lint fixes #294
Conversation
@@ -27,6 +27,7 @@ const addressApi = { | |||
if (res && res.data) { | |||
return res.data; | |||
} | |||
return undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return undefined; | |
return; |
Would this also work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It wouldn't: removing the explicit undefined
raises an error about an unnecessary return
on the last line of the function, and removing the return
breaks the consistent-return
lint rule that I intended to fix in the first time.
Maybe we can find a better solution at another time, but for now I'd rather not play with code logic.
src/api/customAxiosInstance.js
Outdated
* @param {number} timeout Timeout in milliseconds for the request | ||
* @param {number} [_timeout] Timeout in milliseconds for the request | ||
*/ | ||
const createRequestInstance = (resolve, timeout) => { | ||
const createRequestInstance = (resolve, _timeout) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion(if-minor): Should we remove timeout?
If its not used we could just remove this argument, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Removed on dcadfa5
f38cf8e
to
f631cec
Compare
f631cec
to
dcadfa5
Compare
}) | ||
.catch(error => { | ||
.catch(_error => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't you do:
.catch(_error => { | |
.catch(() => { |
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather keep the variable, so that we feel more inclined to properly handle these errors in the future
Acceptance Criteria
Security Checklist