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

Allow null to Peer.AppProtocolVersion for easy configuration #280

Merged
merged 5 commits into from Jun 19, 2019

Conversation

Projects
None yet
3 participants
@longfin
Copy link
Member

commented Jun 11, 2019

This patch allows null value to Peer.AppProtocolVersion. so this adds version-less Peer constructor and Peer.WithAppProtocolVersion().

Also, this PR widens the line length limit to 100 (as #222).

@codecov

This comment has been minimized.

Copy link

commented Jun 11, 2019

Codecov Report

Merging #280 into master will increase coverage by 0.05%.
The diff coverage is 96.38%.

@@            Coverage Diff             @@
##           master     #280      +/-   ##
==========================================
+ Coverage   87.26%   87.31%   +0.05%     
==========================================
  Files         185      185              
  Lines       12136    12204      +68     
==========================================
+ Hits        10590    10656      +66     
  Misses       1307     1307              
- Partials      239      241       +2
Impacted Files Coverage Δ
Libplanet.Tests/Net/PeerTest.cs 100% <100%> (ø) ⬆️
Libplanet/Net/Peer.cs 92.72% <100%> (+1.81%) ⬆️
Libplanet/Net/Swarm.cs 74.1% <57.14%> (-0.12%) ⬇️

@longfin longfin force-pushed the longfin:feature/peer-without-version branch 3 times, most recently from 9471589 to 75f6ed1 Jun 14, 2019

@longfin longfin force-pushed the longfin:feature/peer-without-version branch from 75f6ed1 to 630e12b Jun 19, 2019

@longfin longfin changed the title (WIP) Allow null to Peer.AppProtocolVersion for easy configuration Allow null to Peer.AppProtocolVersion for easy configuration Jun 19, 2019

@longfin longfin force-pushed the longfin:feature/peer-without-version branch from 630e12b to 45ef4af Jun 19, 2019

@longfin longfin requested review from dahlia and earlbread Jun 19, 2019

@longfin longfin marked this pull request as ready for review Jun 19, 2019

Show resolved Hide resolved Libplanet/Net/Peer.cs Outdated
@@ -79,7 +94,7 @@ protected Peer(SerializationInfo info, StreamingContext context)
/// <seealso cref="Swarm.DifferentVersionPeerEncountered"/>
[IgnoreDuringEquals]
[Pure]
public int AppProtocolVersion { get; }
public int? AppProtocolVersion { get; }

This comment has been minimized.

Copy link
@dahlia

dahlia Jun 19, 2019

Member

As this breaks the existing interface (IL-wise at least) we should add this to CHANGES.md's Backward-incompatible interface changes section.

Show resolved Hide resolved CHANGES.md Outdated
Show resolved Hide resolved CHANGES.md Outdated

longfin and others added some commits Jun 19, 2019

Apply suggestions from code review
Co-Authored-By: Hong Minhee <hong.minhee@gmail.com>
Apply suggestions from code review
Co-Authored-By: Hong Minhee <hong.minhee@gmail.com>

@longfin longfin requested a review from dahlia Jun 19, 2019

@dahlia

dahlia approved these changes Jun 19, 2019

@longfin longfin merged commit aa37909 into planetarium:master Jun 19, 2019

16 checks passed

Travis CI - Pull Request Build Passed
Details
WIP Ready for review
Details
changelog This contains self-describing changelog.
Details
codecov/patch Coverage not affected when comparing 7319122...da09a3a
Details
codecov/project 87.31% (+<.01%) compared to 7319122
Details
docs Libplanet docs generated by DocFX
Details
planetarium.libplanet Build #20190619.8 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.