-
Notifications
You must be signed in to change notification settings - Fork 17
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
Use polarity
instead of negative
for Lit's public API
#54
Conversation
bors r+ |
54: Use `polarity` instead of `negative` for Lit's public API r=jix a=jix This resolves #53 Co-authored-by: Jannis Harder <[email protected]>
Lit::from_litidx(var.index, negative) | ||
/// Creates a literal from a `Var` and a `bool` that is `true` when the literal is positive. | ||
pub fn from_var(var: Var, polarity: bool) -> Lit { | ||
Lit::from_litidx(var.index, polarity) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a nice ergonomic improvement, and a semver breaking change. The code like Lit::from_var(x, false)
compiles before and after but means exactly opposite things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not semver breaking as I'm still on 0.x.y. Maybe I should add a more explicit warning that the API isn't stable yet. I will release a version 1.0 when I'm done tweaking the API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cargo disagrees with the semver spec about this:
This compatibility convention is different from SemVer in the way it treats versions before 1.0.0. While SemVer says there is no compatibility before 1.0.0, Cargo considers 0.x.y to be compatible with 0.x.z, where y ≥ z and x > 0.
So if you have a Cargo.toml that has varisat = "0.2"
then you may get 0.2.0
or 0.2.1
as cargo consiters them semver compatible. Ether version will compile Lit::from_var(x, false)
but do opposite things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I wasn't aware of that, I would have released this as 0.3.0 otherwise. Thanks for pointing out the details.
This resolves #53