search

dry-rb / dry-logic

Predicate logic with rule composition
https://dry-rb.org/gems/dry-logic/
MIT License
179 stars 66 forks source link

New `nil?` method break API #41

Closed oded-zahavi closed 5 years ago

oded-zahavi commented 5 years ago

Hi,

When upgrading trailblazer I got dry-logic upgraded as well. As part of the upgrade, method none? became an alias to a new method nil?, which implements the same logic.I assume this alias was there in order to keep backward compatibility and not to break the API: https://github.com/dry-rb/dry-logic/compare/v0.4.2..v0.5.0#diff-e2e34652eb2e31cc606bab1f22f63c82R17

However, This breaks Ruby's nil method: If calling applications called nil? assuming they are calling Ruby's nil? method, they now call dry-logic's nil? method, which requires one argument thus fails:

.../gems/dry-logic-0.5.0/lib/dry/logic/predicates.rb:17:in `nil?': wrong number of arguments (given 0, expected 1) (ArgumentError
.../gems/dry-configurable-0.7.0/lib/dry/configurable.rb:111:in `setting'
.../gems/dry-validation-0.10.7/lib/dry/validation/schema/class_interface.rb:14:in `<class:Schema>'
.../gems/dry-validation-0.10.7/lib/dry/validation/schema/class_interface.rb:7:in `<module:Validation>'
.../gems/dry-validation-0.10.7/lib/dry/validation/schema/class_interface.rb:6:in `<module:Dry>'
.../gems/dry-validation-0.10.7/lib/dry/validation/schema/class_interface.rb:5:in `<top (required)>'
.../gems/dry-validation-0.10.7/lib/dry/validation/schema.rb:15:in `require'
.../gems/dry-validation-0.10.7/lib/dry/validation/schema.rb:15:in `<top (required)>'
.../gems/dry-validation-0.10.7/lib/dry/validation.rb:39:in `require'
.../gems/dry-validation-0.10.7/lib/dry/validation.rb:39:in `<top (required)>'
.../gems/dry-validation-0.10.7/lib/dry-validation.rb:1:in `require'
.../gems/dry-validation-0.10.7/lib/dry-validation.rb:1:in `<top (required)>'
solnic commented 5 years ago

This is fixed in dry-validation 0.13.0 and there's also 0.12.3 which doesn't pull in dry-logic 0.5.0. Can you upgrade dry-v?

oded-zahavi commented 5 years ago

Hey thanks for the very prompt reply. Upgrading dry-validation seems to fix the issue. However, imho this is still a breaking API change requiring a major version upgrade, since the two gems are decoupled. Up to you.

flash-gordon commented 5 years ago

Not sure what you mean, dry-logic 0.5 is a major release, so is dry-validation 0.13

TiteiKo commented 5 years ago

That's a question I've had for quite a time, do you use https://semver.org/ ? (note that as the gem hasn't reached 1.0.0 yet, any version change can break everything according to semver)

If not, could you explain your versioning scheme somewhere?

flash-gordon commented 5 years ago

0.x bump means a major version bump, 0.x.y bump is a minor, backward compatible change. This is done according to how the ~> operator works

oded-zahavi commented 5 years ago

x.Y.z is usually considered a minor version (added functionality) in x.y.z scheme. x.y.Z is considered a patch/fix, i.e. bug fix (or a change in internal implementation), as @TiteiKo suggested. Per semver (https://semver.org/#spec-item-8), major version (breaking changes) should by X.y.z. I guess this is what caused our bug.

flash-gordon commented 5 years ago

@oded-zahavi these rules are applicable for post-1.0.0 versions. Semver states 0.x.y versions are not considered stable and can break anything at any moment https://semver.org/#spec-item-4 We don't do breaking changes when incrementing y in 0.x.y but incrementing x is a major bump. Once we release 1.0.0 we'll follow the rules you referenced, that's what we do for dry-initializer and dry-monads atm.

oded-zahavi commented 5 years ago

I am well aware. Thanks.

solnic commented 5 years ago

I'm closing this since there's nothing else related to this issue that needs to be done.