enspirit / bmg

Bmg, Alf's successor, A Ruby Relational Algebra
MIT License
234 stars 8 forks source link

make Path a dependency #17

Closed krazerxz closed 4 years ago

krazerxz commented 4 years ago

Trying to load this in a Rails app produces a LoadError.

I think path needs to be included as a dependency (not just dev dependency) because it's referenced in lib/bmg.rb

I was looking for a library that does just this a couple of weeks ago and this just popped up on HN, great stuff!

blambeau commented 4 years ago

Thanks for interest @krazerxz

By chance, have you tried removing the require in bmg.rb instead? I'd prefer not having path as production dependency. I don't think it's actually required, and I don't like the node.js syndrome ;)

blambeau commented 4 years ago

Btw @krazerxz, if you plan to use this in Rails, do you also use Sequel? Otherwise the SQL generator won't work.

I would definitely support (and contribute to) pull requests making Bmg work with Rails (on top of Arel, or directly on top of low-level SQL adapters).

krazerxz commented 4 years ago

By chance, have you tried removing the require in bmg.rb instead?

I thought about that, but Path is used in lib/bmg/sql/grammar.rb.

I've not had time to see where this is called from/if the dependency can be removed. Otherwise, all other usage is just within specs.

krazerxz commented 4 years ago

Hi @blambeau This has been undone by https://github.com/enspirit/bmg/commit/cad3e3941bcdc316fe046fb3f1676d765787eb51

Was this intentional?

blambeau commented 4 years ago

Not intentional and fixed.

Thanks.

krazerxz commented 4 years ago

Great, thanks 👍