dkubb / axiom

Simplifies querying of structured data using relational algebra
https://github.com/dkubb/axiom
MIT License
459 stars 22 forks source link

[WIP] Upgrade equalizer and use adamantium from master #52

Closed solnic closed 10 years ago

solnic commented 10 years ago

This branch upgrades equalizer to 0.0.9 and uses adamatium from master which no longer has the issue described in #51

There's 27 failing tests related to changed implementation of #hash method in equalizer (IIRC). Personally I'd vote for removing those tests altogether since it is a feature coming from Equalizer gem and it is tested there. I don't see any reason why this should be tested here (again, those tests check implementation details, very little value). Alternatively we could have tests for #eql? instead of #hash which would test behavior and not implementation. Yes, I've become a huge BDD fan lately ;)

Another problem is with adamantium from master, even though #51 is no longer a problem with edge adamantium there's a lot of failures ending up with an error like that:

 144) Axiom::Function::Predicate::LessThan#inverse right
   Failure/Error: subject { object.inverse }
   ArgumentError:
     wrong number of arguments (2 for 1)
   # /Users/solnic/.gem/ruby/1.9.3/gems/memoizable-0.4.0/lib/memoizable/instance_methods.rb:32:in `memoize'
   # ./lib/axiom/function/binary.rb:87:in `inverse'
  ...

I guess some public interface in adamantium changed and axiom should be updated. I can do that but I need some guidance.

dkubb commented 10 years ago

@solnic I believe my last commits in master fix this issue. Can you try them out and let me know if the problem you are seeing persists?

I'm going to mark this as closed for now.

solnic commented 10 years ago

@dkubb thanks, looks like everything's fine now!