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

Restore BlockCompletion<T> (#707) and more #798

Merged
merged 10 commits into from Feb 24, 2020

Conversation

@dahlia
Copy link
Member

dahlia commented Feb 21, 2020

The first commit is a commit squashed from the PR #707, except for some adjustments to resolve conflicts and make the build to pass.

This also introduces C# 8's new async streams syntax to replace Dasync's AsyncEnumerator package with it. Although this removed the dependency on AsyncEnumerator package and introduce the new syntax into Libplanet project, note Libplanet.Tests project still uses C# 7.2 and the third-party package, which is intended.

Instead of AsyncEnumerator, Libplanet project has a new dependency on the System.Linq.Async package, which provides several LINQ extension methods laid on IAsyncEnumerator<T>.

The key changes over the reverted PR #707 in this patch are of course fixing #707's bugs and more PreloadStates. See also each commit message.

@dahlia dahlia requested review from longfin, earlbread and moreal Feb 21, 2020
@dahlia dahlia self-assigned this Feb 21, 2020
@dahlia dahlia force-pushed the dahlia:block-completion branch from fa4d2b0 to 0d376eb Feb 21, 2020
@longfin

This comment has been minimized.

Copy link
Member

longfin commented Feb 22, 2020

Tests seem failed. 😢

@dahlia Is it still in progress?

@dahlia dahlia force-pushed the dahlia:block-completion branch from 0d376eb to b80bf36 Feb 22, 2020
@codecov

This comment has been minimized.

Copy link

codecov bot commented Feb 22, 2020

Codecov Report

Merging #798 into master will increase coverage by 0.47%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #798      +/-   ##
==========================================
+ Coverage   86.38%   86.85%   +0.47%     
==========================================
  Files         227      234       +7     
  Lines       19637    20751    +1114     
==========================================
+ Hits        16964    18024    +1060     
- Misses       1398     1428      +30     
- Partials     1275     1299      +24     
Impacted Files Coverage Δ
Libplanet.Tests/Net/Protocols/TestTransport.cs 75.79% <0.00%> (-0.64%) ⬇️
Libplanet/Net/BlockHashDownloadState.cs 100.00% <0.00%> (ø)
Libplanet/Net/BlockCompletion.cs 93.14% <0.00%> (ø)
Libplanet/Net/BlockVerificationState.cs 100.00% <0.00%> (ø)
Libplanet.Tests/Net/BlockCompletionTest.cs 98.51% <0.00%> (ø)
Libplanet.Tests/Blockchain/BlockLocatorTest.cs 100.00% <0.00%> (ø)
Libplanet.Tests/LoggerExtensions.cs 62.06% <0.00%> (ø)
Libplanet.Tests/Net/Messages/BlockHashesTest.cs 93.10% <0.00%> (ø)
Libplanet.Tests/Net/SwarmTest.cs 96.62% <0.00%> (+0.27%) ⬆️
Libplanet/Crypto/PrivateKey.cs 86.20% <0.00%> (+0.86%) ⬆️
... and 1 more
@dahlia dahlia force-pushed the dahlia:block-completion branch 3 times, most recently from cc29203 to e497ea4 Feb 22, 2020
@dahlia

This comment has been minimized.

Copy link
Member Author

dahlia commented Feb 22, 2020

@longfin Fixed the broken tests.

@dahlia dahlia added this to In review in 스프린트서울 202002 Feb 22, 2020
@dahlia

This comment has been minimized.

Copy link
Member Author

dahlia commented Feb 22, 2020

/rebase

@libplanet libplanet force-pushed the dahlia:block-completion branch from e497ea4 to e3d1e84 Feb 22, 2020
@dahlia dahlia requested a review from moreal Feb 22, 2020
Libplanet.Tests/Net/SwarmTest.cs Outdated Show resolved Hide resolved
Libplanet/Net/BlockCompletion.cs Outdated Show resolved Hide resolved
var completion =
new ConcurrentDictionary<HashDigest<SHA256>, bool>(_satisfiedBlocks);

await foreach (var hashes in EnumerateChunks(cancellationToken))

This comment has been minimized.

Copy link
@longfin

longfin Feb 22, 2020

Member

IMHO, it would be better if we specify type even async foreach.

Suggested change
await foreach (var hashes in EnumerateChunks(cancellationToken))
await foreach (IEnumerable<HashDigest<SHA256>> hashes in EnumerateChunks(cancellationToken))

This comment has been minimized.

Copy link
@dahlia

dahlia Feb 24, 2020

Author Member

I tried in that way first, but turned that into this way because the type made the line too long (> 100 columns). 😩

Libplanet/Net/BlockCompletion.cs Outdated Show resolved Hide resolved
Libplanet/Net/Swarm.cs Outdated Show resolved Hide resolved
Libplanet/Net/Swarm.cs Outdated Show resolved Hide resolved
@dahlia dahlia force-pushed the dahlia:block-completion branch from e3d1e84 to 897b3a3 Feb 24, 2020
@dahlia dahlia force-pushed the dahlia:block-completion branch from 897b3a3 to cd8dfc5 Feb 24, 2020
@dahlia dahlia requested a review from earlbread Feb 24, 2020
@dahlia dahlia force-pushed the dahlia:block-completion branch from cd8dfc5 to 2a13ae7 Feb 24, 2020
@dahlia dahlia requested a review from longfin Feb 24, 2020
@moreal
moreal approved these changes Feb 24, 2020
@dahlia dahlia merged commit 93fd48b into planetarium:master Feb 24, 2020
20 checks passed
20 checks passed
benchmarks (macos-latest)
Details
dist
Details
benchmarks (ubuntu-18.04)
Details
benchmarks (windows-latest)
Details
docs
Details
WIP Ready for review
Details
changelog This contains self-describing changelog.
Details
codecov/patch 91.22% of diff hit (target 86.38%)
Details
codecov/project 86.85% (+0.47%) compared to aea1682
Details
license/cla Contributor License Agreement is signed.
Details
planetarium.libplanet Build #20200224.6 succeeded
Details
planetarium.libplanet (Linux_Mono) Linux_Mono succeeded
Details
planetarium.libplanet (Linux_NETCore) Linux_NETCore succeeded
Details
planetarium.libplanet (Windows_Mono) Windows_Mono succeeded
Details
planetarium.libplanet (Windows_NETCore) Windows_NETCore succeeded
Details
planetarium.libplanet (Windows_NETCore_coverage) Windows_NETCore_coverage succeeded
Details
planetarium.libplanet (Windows_NETFramework) Windows_NETFramework succeeded
Details
planetarium.libplanet (macOS_Mono) macOS_Mono succeeded
Details
planetarium.libplanet (macOS_NETCore) macOS_NETCore succeeded
Details
planetarium.libplanet (macOS_Unity) macOS_Unity succeeded
Details
스프린트서울 202002 automation moved this from In review to Done Feb 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.