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

refactor(trie): replace semaphore-async-await with simpler implementation #2187

Merged
merged 1 commit into from
Aug 19, 2022

Conversation

faustbrian
Copy link
Contributor

@faustbrian faustbrian commented Aug 19, 2022

Drops the semaphore-async-await dependency and replaces it with only the bits that we're actually using (which is very little). I left comments with links to the original implementations in the code.

The trie package is now almost free of third-party dependencies, only the readable-stream remains for an unused class. Assuming #2167 gets merged.

@codecov
Copy link

codecov bot commented Aug 19, 2022

Codecov Report

Merging #2187 (cec5757) into master (cec5757) will not change coverage.
The diff coverage is n/a.

❗ Current head cec5757 differs from pull request most recent head 805e92c. Consider uploading reports for the commit 805e92c to get more accurate results

Impacted file tree graph

Flag Coverage Δ
block 93.07% <0.00%> (ø)
blockchain 88.82% <0.00%> (ø)
client 86.67% <0.00%> (ø)
common 98.09% <0.00%> (ø)
devp2p 92.33% <0.00%> (ø)
ethash ∅ <0.00%> (∅)
evm 79.07% <0.00%> (ø)
rlp ∅ <0.00%> (∅)
statemanager 88.71% <0.00%> (ø)
trie 89.29% <0.00%> (ø)
tx 97.98% <0.00%> (ø)
util 93.14% <0.00%> (ø)
vm 85.27% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@faustbrian faustbrian changed the title refactor: replace semaphore-async-await with simpler implementation refactor(trie): replace semaphore-async-await with simpler implementation Aug 19, 2022
@holgerd77 holgerd77 force-pushed the semaphore-async-await branch from 85e90bd to 805e92c Compare August 19, 2022 07:52
@holgerd77
Copy link
Member

Rebased this via UI.

Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

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

Pretty cool, LGTM, just checked, dependency is MIT licensed, so this should be ok. 👍 🙂

@holgerd77 holgerd77 merged commit d697a53 into master Aug 19, 2022
@holgerd77 holgerd77 deleted the semaphore-async-await branch August 19, 2022 08:42
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