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

Fix Rfc1918ClassBPrivateSpec incorrect matches #796

Conversation

improved-broccoli
Copy link

Fix #795

@improved-broccoli
Copy link
Author

A test failed, but it does not seem related to my changes:

[info] + RefTypeRead.load success: OK, proved property.
1179Error: Option --foo failed when given '0'. Predicate failed: (0 > 0).
1180Usage: tests [options]
1181
1182  -f, --foo <value>  foo is a positive integer property
1183[info] + RefTypeRead.load failure (predicate): OK, proved property.
1184Error: Option --foo expects a number but was given 'abc'
1185Usage: tests [options]
1186
1187  -f, --foo <value>  foo is a positive integer property

It already happened on master.

@fthomas
Copy link
Owner

fthomas commented Aug 13, 2020

I think the error that failed the build is:

[warn] /home/travis/build/fthomas/refined/modules/core/shared/src/main/scala/eu/timepit/refined/types/net.scala isn't formatted properly!
[warn] /home/travis/build/fthomas/refined/modules/core/shared/src/main/scala/eu/timepit/refined/types/net.scala isn't formatted properly!
...
[error] 1 files must be formatted
[error] 1 files must be formatted
[error] (coreJS / Compile / scalafmtCheck) 1 files must be formatted
[error] (coreJVM / Compile / scalafmtCheck) 1 files must be formatted

Running sbt scalafmtAll should fix the formatting.

…ts with characters in rfc allowed range but still outside of it
@improved-broccoli improved-broccoli force-pushed the incorrect-matches-rfc1918classbprivatespec branch from 580c198 to f863e6f Compare August 13, 2020 18:58
@improved-broccoli
Copy link
Author

improved-broccoli commented Aug 13, 2020

Didn't know about scalafmtAll command, thanks for telling me.
I push force my changes to avoid a squash, do you mind?
Everything seems ok now.
When do you plan the next release including this fix?
Thanks.

Copy link
Owner

@fthomas fthomas left a comment

Choose a reason for hiding this comment

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

LGTM now. Many thanks @jbenoit2011!

@fthomas fthomas merged commit 1c09e58 into fthomas:master Aug 14, 2020
@fthomas
Copy link
Owner

fthomas commented Aug 14, 2020

I can make a release with this fix soon. But before I do, it may be worth looking into another issue with Rfc1918ClassBPrivateSpec that is mentioned in #702:

There is also a bug in Rfc1918ClassBPrivateSpec. It should start at 172.16.0.0 not 172.15.0.0. https://tools.ietf.org/html/rfc1918#section-3

WDYT @jbenoit2011?

@improved-broccoli
Copy link
Author

improved-broccoli commented Aug 14, 2020

I just pushed a patch to fix the start range of rfc1918 class b ips issue described in #702
EDIT: branch has been merged while I was pushing this new commit :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rfc1918ClassBPrivateSpec incorrect matches
2 participants