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: Least intrusive lint rules #679

Merged
merged 26 commits into from
Jun 28, 2024
Merged

Conversation

tuliomir
Copy link
Contributor

@tuliomir tuliomir commented Jun 17, 2024

Acceptance Criteria

  • Fix linting errors and warnings that cause the least impact possible on the code
  • Each commit should handle a single linting rule to facilitate the reviewing process
  • Each eslint-disable-* command should have a comment explaining why it was used

Note

To fix the no-undef rule it was necessary to upgrade tsconfig.json target from es6 to es2018.

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 Jun 17, 2024
Base automatically changed from chore/eslint-autofixes to master June 19, 2024 14:36
@tuliomir tuliomir force-pushed the style/lint-least-intrusive branch from a804c6b to 58b352e Compare June 19, 2024 14:41
Copy link

codecov bot commented Jun 19, 2024

Codecov Report

Attention: Patch coverage is 84.09091% with 14 lines in your changes missing coverage. Please review.

Project coverage is 79.69%. Comparing base (fda9321) to head (c174621).

Files Patch % Lines
src/wallet/wallet.ts 0.00% 5 Missing ⚠️
src/wallet/websocket.ts 0.00% 3 Missing ⚠️
src/api/metadataApi.ts 0.00% 2 Missing ⚠️
src/api/axiosWrapper.ts 80.00% 1 Missing ⚠️
src/utils/transaction.ts 0.00% 1 Missing ⚠️
src/wallet/sendTransactionWalletService.ts 0.00% 1 Missing ⚠️
src/websocket/base.ts 50.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master     #679       +/-   ##
===========================================
+ Coverage   62.82%   79.69%   +16.87%     
===========================================
  Files          77       77               
  Lines        5928     5935        +7     
  Branches     1284     1286        +2     
===========================================
+ Hits         3724     4730     +1006     
+ Misses       2106     1187      -919     
+ Partials       98       18       -80     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tuliomir tuliomir requested review from andreabadesso and r4mmer June 19, 2024 14:45
@tuliomir tuliomir marked this pull request as ready for review June 19, 2024 14:45
@tuliomir tuliomir requested a review from pedroferreira1 as a code owner June 19, 2024 14:45
@tuliomir tuliomir removed the request for review from pedroferreira1 June 19, 2024 15:47
resolve?: Function | null,
timeout?: number | null,
additionalHeaders = {}
resolve?: undefined | null, // XXX: We should remove or use this parameter
Copy link
Member

Choose a reason for hiding this comment

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

We should remove if it is not used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would be a breaking change to a method that is used in many places, both in the lib and in its clients: I took the path of just mapping the situation instead of acting now.

Do you agree with this approach?

r4mmer
r4mmer previously approved these changes Jun 26, 2024
andreabadesso
andreabadesso previously approved these changes Jun 26, 2024
@tuliomir tuliomir dismissed stale reviews from andreabadesso and r4mmer via 44bea46 June 28, 2024 13:01
@tuliomir tuliomir force-pushed the style/lint-least-intrusive branch from de3e775 to 44bea46 Compare June 28, 2024 13:01
@tuliomir tuliomir merged commit b751104 into master Jun 28, 2024
4 checks passed
@tuliomir tuliomir deleted the style/lint-least-intrusive branch June 28, 2024 16:11
r4mmer added a commit that referenced this pull request Jun 29, 2024
…-config

* origin/master:
  style: Least intrusive lint rules (#679)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants