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

Tests: IsReachable is the inverse of IsLimited (DRY). Includes unit tests #15051

Merged
merged 1 commit into from Jan 9, 2019

Conversation

Projects
None yet
6 participants
@mmachicao
Copy link
Contributor

commented Dec 28, 2018

IsReachable is the inverse of IsLimited, but the implementation is duplicated (DRY)

  • Changed the implementation accordingly.
  • Added unit tests to document behavior and relationship
  • My modification in net.cpp applies only to IsReachable.
  • Applied clang-format-diffpy

Created new pull request to avoid the mess with:
#15044

Checked with supposedly conflicting PRs mentioned in the old PR. No conflicts with the specific changes in this PR.

@fanquake fanquake added the Tests label Dec 28, 2018

@Empact

This comment has been minimized.

Copy link
Member

commented Dec 30, 2018

utACK 6dc4593

@promag
Copy link
Member

left a comment

utACK 6dc4593.

another nit, could have separate commits.

BOOST_CHECK_EQUAL(IsReachable(NET_INTERNAL), true);
}

CNetAddr UtilBuildAddress(unsigned char p1, unsigned char p2, unsigned char p3, unsigned char p4)

This comment has been minimized.

Copy link
@promag

promag Dec 31, 2018

Member

nit, static.

@laanwj

This comment has been minimized.

Copy link
Member

commented Jan 2, 2019

I thought they were different. Did this change at some point?
In any case with the current code this looks correct.
utACK.

@Empact

This comment has been minimized.

Copy link
Member

commented Jan 2, 2019

Have been the same since #7553

BOOST_CHECK_EQUAL(IsReachable(NET_IPV6), false);
BOOST_CHECK_EQUAL(IsReachable(NET_ONION), false);


This comment has been minimized.

Copy link
@promag

promag Jan 4, 2019

Member

nit, remove 2nd new line.

@practicalswift

This comment has been minimized.

Copy link
Member

commented Jan 5, 2019

utACK 6dc4593

Warm welcome as a contributor! I look forward to more test improvements from you :-)

@laanwj

This comment has been minimized.

Copy link
Member

commented Jan 9, 2019

I'm not going to block merging this on an extra newline.

@laanwj laanwj merged commit 6dc4593 into bitcoin:master Jan 9, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Jan 9, 2019

Merge #15051: Tests: IsReachable is the inverse of IsLimited (DRY). I…
…ncludes unit tests

6dc4593 IsReachable is the inverse of IsLimited (DRY). Includes unit tests (marcaiaf)

Pull request description:

  IsReachable is the inverse of IsLimited, but the implementation is duplicated (DRY)

  - Changed the implementation accordingly.
  - Added unit tests to document behavior and relationship
  - My modification in net.cpp  applies only to IsReachable.
  - Applied clang-format-diffpy

  Created new pull request to avoid the mess with:
  #15044

  Checked with supposedly conflicting PRs mentioned in the old PR. No conflicts with the specific changes in this PR.

Tree-SHA512: b132dec6cc2c788ebe4f63f228d78f441614e156743b17adebc990de0180a5872874d2724c86eeaa470b4521918bd137b0e33ebcaae77c5efc1f0d56104f6c87

laanwj added a commit that referenced this pull request Jan 14, 2019

Merge #15138: Drop IsLimited in favor of IsReachable
d6b076c Drop IsLimited in favor of IsReachable (Ben Woosley)

Pull request description:

  These two methods have had the same meaning, but inverted, since
  110b62f. Having one name for a single
  concept simplifies the code.

  This is a follow-up to #15051.
  /cc #7553

Tree-SHA512: 347ceb9e2a55ea06f4c01226411c7bbcade09dd82130e4c59d0824ecefd960875938022edbe5d4bfdf12b0552c9b4cb78b09a688284d707119571daf4eb371b4
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.