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.

Sign up for GitHub

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

Jump to bottom

fix: add eslint rule require await #842

Merged
merged 26 commits into from
Mar 29, 2023
Merged

fix: add eslint rule require await #842

merged 26 commits into from
Mar 29, 2023

Conversation

Torres-ssf
Copy link
Contributor

@Torres-ssf Torres-ssf commented Mar 21, 2023 •
edited
Loading

@Torres-ssf
add eslint rule require-await
82a7dd6
@Torres-ssf
disable require-await for encrypt and decrypt
179391b
@Torres-ssf
disable require-await for InvocationCallResult build
f78eb6a
@Torres-ssf
made getNetwork conform with require-await
8ac8ac0
@Torres-ssf
made Wallet lock method conform with require-await
0ac9b1b
@Torres-ssf
made MemoryStorage methods conform with require-await
de682e6
@Torres-ssf
disable require-await for scripts
171ce29
@Torres-ssf
made BaseWalletUnlocked conform with require-await
8747cd2
@Torres-ssf
remove unnecessary async from abi-typegen tests
c287930
@Torres-ssf
remove unnecessary async from address tests
4bcdb91
@Torres-ssf
remove unnecessary async from fuel-gauge tests
2985a87
@Torres-ssf
remove unnecessary async from fuels tests
d6da3ad
@Torres-ssf
remove unnecessary async from versions tests
c3c52d7
@Torres-ssf
remove unnecessary async from wallet tests
88530f2
@Torres-ssf
remove unnecessary async from wallet-manager tests
ae96cba
@Torres-ssf
remove unnecessary async from other packages tests
dbdfa96
@Torres-ssf
refact unit tests
86f66f2
@Torres-ssf
add empty changeset
4174035
@Torres-ssf Torres-ssf self-assigned this Mar 21, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Mar 21, 2023 •
edited
Loading

Coverage report

St.❔
 Category Percentage Covered / Total
🟢 Statements 95% 4564/4804
🟢 Branches 83.55% 818/979
🟢 Functions 92% 886/963
🟢 Lines 94.89% 4384/4620

Test suite run success

760 tests passing in 109 suites.

Report generated by 🧪jest coverage report action from 7add490

@Torres-ssf Torres-ssf marked this pull request as ready for review March 21, 2023 15:53
arboleya
arboleya requested changes Mar 21, 2023
View reviewed changes
Copy link
Member

@arboleya arboleya left a comment

Choose a reason for hiding this comment

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

Well done. 👌

The incoherence that can creep in with just one missing eslint rule is huge.

I left a lot of questions in an attempt of trying to understand the rationale for it all.

There's also this.

scripts/changeset-version-with-docs.ts Outdated Show resolved Hide resolved
scripts/forc-update.ts Outdated Show resolved Hide resolved
packages/wallet/src/base-unlocked-wallet.ts Outdated Show resolved Hide resolved
packages/keystore/src/aes-ctr-node.ts Outdated Show resolved Hide resolved
packages/keystore/src/aes-ctr-node.ts Outdated Show resolved Hide resolved
packages/providers/src/provider.test.ts Show resolved Hide resolved
packages/providers/src/provider.ts Show resolved Hide resolved
packages/wallet-manager/src/storages/memory-storage.ts Outdated Show resolved Hide resolved
packages/wallet-manager/src/wallet-manager.ts Outdated Show resolved Hide resolved
packages/wallet/src/base-unlocked-wallet.ts Outdated Show resolved Hide resolved
camsjams
camsjams reviewed Mar 24, 2023
View reviewed changes
Copy link
Contributor

@camsjams camsjams left a comment

Choose a reason for hiding this comment

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

Looks good so far, awaiting to see what the open items are being resolved with.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 27, 2023 •
edited
Loading

Coverage report

St.❔
 Category Percentage Covered / Total
🟢 Statements
95.03% (+0% 🔼)
 4593/4833
🟢 Branches 83.59% 820/981
🟢 Functions 92.02% 888/965
🟢 Lines
94.92% (+0% 🔼)
 4410/4646

Test suite run success

764 tests passing in 110 suites.

Report generated by 🧪jest coverage report action from 13448b2

@Torres-ssf Torres-ssf enabled auto-merge (squash) March 27, 2023 16:40
This was linked to issues Mar 27, 2023
Possibly new flaky test #845
Closed
Investigate and adopt eslint rule require-await #832
Closed
arboleya
arboleya previously approved these changes Mar 27, 2023
View reviewed changes
camsjams
camsjams previously approved these changes Mar 28, 2023
View reviewed changes
@Torres-ssf Torres-ssf dismissed stale reviews from camsjams and arboleya via 3ae4159 March 28, 2023 13:18
@Torres-ssf
Copy link
Contributor Author

@arboleya @camsjams I mistakenly updated the branch with a push and all reviews were dismissed. Nothing changed here

@Torres-ssf Torres-ssf merged commit 6046e4e into master Mar 29, 2023
@Torres-ssf Torres-ssf deleted the torres/fix/add-eslint-require-await branch March 29, 2023 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arboleya arboleya arboleya approved these changes

@Dhaiwat10 Dhaiwat10 Dhaiwat10 approved these changes

@luizstacio luizstacio Awaiting requested review from luizstacio

@LuizAsFight LuizAsFight Awaiting requested review from LuizAsFight

@camsjams camsjams Awaiting requested review from camsjams

Assignees

@Torres-ssf Torres-ssf

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Possibly new flaky test Investigate and adopt eslint rule require-await
4 participants
@Torres-ssf @arboleya @camsjams @Dhaiwat10