Skip to content
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

Import connect into query, connect docstring improvements #44

Merged
merged 3 commits into from
Jul 1, 2015

Conversation

danielcompton
Copy link
Collaborator

It is often a pain to have to import the rethinkdb.core namespace just to make a connection. This PR imports the connect function into the rethinkdb.query ns which is the most common ns for API consumers to be using.

I wasn't keen on using something like Potemkin for a single function, sing out if you've got alternative approaches or have an issue with this, otherwise I'll merge this in a few days.

  • Add documentation
  • Update tests to use new connect function?

@danielcompton danielcompton self-assigned this Jun 29, 2015
@danielcompton danielcompton changed the title Import connect into query, docstring improvements Import connect into query, connect docstring improvements Jun 30, 2015
@danielcompton
Copy link
Collaborator Author

I have no earthly idea why 441790f fixes the tests, would love for someone else to have a look and tell me why.

@danielcompton
Copy link
Collaborator Author

There may be some conflicts between the close created by the defrecord, and the close function that we define, although I think there's something deeper there too.

@yurrriq
Copy link
Contributor

yurrriq commented Jun 30, 2015

@danielcompton, on line 27 there's

(def close ...) ; <= for connections

but then on line 34 there's

(defn close ...) ; <= for cursors

So when you

(:require [rehtinkdb.query :as r])

;; ...

(r/close ...)

it refers to the cursor-closing function.

That's my guess anyway.

@yurrriq
Copy link
Contributor

yurrriq commented Jun 30, 2015

Confirmed:

(in-ns 'rethinkdb.query)
(doc close)
-------------------------
rethinkdb.query/close
([cursor])
  nil
nil

@yurrriq
Copy link
Contributor

yurrriq commented Jun 30, 2015

Since query/close (the cursor-closing function) simple calls net/close which is just sugar for java.io.Closeable's close, maybe the one in query is overkill..

@danielcompton
Copy link
Collaborator Author

After fixing #50, there's no need to add a query/close function here. I've removed the commits and rebased against master.

danielcompton added a commit that referenced this pull request Jul 1, 2015
Import `connect` into query, connect docstring improvements
@danielcompton danielcompton merged commit 2cde5a1 into master Jul 1, 2015
@danielcompton danielcompton deleted the connect-in-query branch July 1, 2015 01:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants