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

Only import propBoolean from Prop #133

Merged
merged 3 commits into from
Jan 24, 2020

Conversation

travisbrown
Copy link
Contributor

I noticed this because of a change in Dotty that I think is a bug, but I think it's the right thing to do anyway, since importing Prop._ brings in a lot of other definitions that aren't needed.

@travisbrown
Copy link
Contributor Author

I pushed a commit changing the JDK for Travis CI to avoid Oracle's (which hasn't been available on Travis CI for a while).

@fthomas
Copy link
Collaborator

fthomas commented Jan 21, 2020

I agree that only importing definitions that are needed is generally better. Prop._ also includes the AnyOperators implicit conversion which I believe allows to write properties like this (notice the ?=):

prop> (i: Int) => Test.f(i) ?= (i * 2)

instead of:

prop> (i: Int) => Test.f(i) == (i * 2)

Maybe AnyOperators should be imported too, to minimize breaking existing doctests.
WDYT @travisbrown?

@travisbrown
Copy link
Contributor Author

@fthomas The problem is that if we don't use the wildcard import then we'll get an unused import warning / failure for AnyOperators. I've never used prop> with sbt-doctest, just the scala> ... res0: ... style. I guess we could use import org.scalacheck.Prop.{BooleanOperators => _, _}?

@fthomas
Copy link
Collaborator

fthomas commented Jan 22, 2020

import org.scalacheck.Prop.{BooleanOperators => _, _} seems fine to me.

@travisbrown
Copy link
Contributor Author

@fthomas Sounds good, done!

@travisbrown
Copy link
Contributor Author

@tkawachi Hi! Any chance we could get a release with this change and #132? I'm happy to help in any way I can.

@tkawachi tkawachi merged commit c0b4d6c into tkawachi:master Jan 24, 2020
@tkawachi
Copy link
Owner

@travisbrown Thank you for the PR. Please try 0.9.6 which I just published.

@travisbrown
Copy link
Contributor Author

@tkawachi I just tried it out in a project where I was seeing warnings, and they're gone. Thanks much!

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.

3 participants