Skip to content

Use mongo driver directly instead of Origin gem#22

Open
estolfo wants to merge 3 commits into
rom-rb:masterfrom
estolfo:use-driver
Open

Use mongo driver directly instead of Origin gem#22
estolfo wants to merge 3 commits into
rom-rb:masterfrom
estolfo:use-driver

Conversation

@estolfo
Copy link
Copy Markdown

@estolfo estolfo commented Jun 30, 2017

No description provided.

Comment thread spec/unit/relation_spec.rb Outdated
to eql([{name: 'Jane',}, {name: 'Joe'}])

expect(users.order(name: :desc).only(:name).without(:_id).to_a.map(&:to_h)).
expect(users.order(name: Mongo::Index::DESCENDING).only(:name).without(:_id).to_a.map(&:to_h)).
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it'd be better if we hide this behind our API, so basically Mongo::Dataset#order could translate :desc and :asc to corresponding driver constants

@jfromell
Copy link
Copy Markdown

jfromell commented Feb 5, 2018

Was this abandoned? 'Cause all that is making the checks fail is a change in ROM::CommandRegistry.

@solnic
Copy link
Copy Markdown
Member

solnic commented Feb 5, 2018

@jfromell looks like it was

@estolfo
Copy link
Copy Markdown
Author

estolfo commented Feb 5, 2018

I'm happy to continue work on this if you'd like. Let me know!

@jfromell
Copy link
Copy Markdown

jfromell commented Feb 6, 2018

The checks will pass again if you merge #25, so it's an easy fix. Any particular reason as to why that pull request hasn't been merged yet?

@estolfo
Copy link
Copy Markdown
Author

estolfo commented Feb 6, 2018

@jfromell I don't know why this wasn't merged. I can keep working on it if the maintainers are interested in having me do so.

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