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

getSeriesNames without defining a database name if defined on connection #19

Merged
merged 2 commits into from
Feb 24, 2014

Conversation

lafikl
Copy link

@lafikl lafikl commented Feb 24, 2014

Hi @bencevans

This pull-request is to make databaseName optional when it's defined on connection, but you can overwrite it if want to.

I really think it's better this way.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 1950424 on lafikl:master into 74c6d84 on bencevans:master.

@nichdiekuh
Copy link
Collaborator

Hi @lafikl,

your suggestion makes sense indeed.
I see you have also removed the "query" parameter from getSeriesUrl()(which is fine, since it's not used), however all calls for this function still contain the query parameter. If you could fix the function calls, removing the query param, I'd be happy to merge your changes.

@lafikl
Copy link
Author

lafikl commented Feb 24, 2014

@nichdiekuh There's only one call for seriesUrl() and i removed query from it's params
I hope everything is good to merge now?

@bencevans
Copy link
Member

Looks good to me, thanks @lafikl. I'll leave @nichdiekuh to take another look too though!

@nichdiekuh
Copy link
Collaborator

Oh, I was seeing ghosts this morning. You're right, there was only one call for this function. Looks good to me too. Thank you @lafikl :-)

nichdiekuh added a commit that referenced this pull request Feb 24, 2014
getSeriesNames without defining a database name if defined on connection
@nichdiekuh nichdiekuh merged commit 6a75aa9 into node-influx:master Feb 24, 2014
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.

None yet

4 participants