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

style: Manual lint fixes #294

Merged
merged 4 commits into from
Aug 21, 2024
Merged

style: Manual lint fixes #294

merged 4 commits into from
Aug 21, 2024

Conversation

tuliomir
Copy link
Contributor

@tuliomir tuliomir commented Aug 19, 2024

Acceptance Criteria

  • Fix all lint warnings and errors on this application
  • The only exception is the parsing error found on most screens: that will only be solved by migrating them to functional components

Security Checklist

  • Make sure you do not include new dependencies in the project unless strictly necessary and do not include dev-dependencies as production ones. More dependencies increase the possibility of one of them being hijacked and affecting us.

@tuliomir tuliomir self-assigned this Aug 19, 2024
@tuliomir tuliomir requested a review from r4mmer as a code owner August 19, 2024 15:00
r4mmer
r4mmer previously approved these changes Aug 19, 2024
@@ -27,6 +27,7 @@ const addressApi = {
if (res && res.data) {
return res.data;
}
return undefined;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return undefined;
return;

Would this also work?

Copy link
Contributor Author

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.

Comment on lines 20 to 22
* @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) => {
Copy link
Member

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?

Copy link
Contributor Author

@tuliomir tuliomir Aug 20, 2024

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

})
.catch(error => {
.catch(_error => {
Copy link
Contributor

Choose a reason for hiding this comment

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

can't you do:

Suggested change
.catch(_error => {
.catch(() => {

?

Copy link
Contributor Author

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

@tuliomir tuliomir merged commit 8437858 into dev Aug 21, 2024
1 check passed
@tuliomir tuliomir deleted the style/lint-fixes branch August 21, 2024 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants