Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add pagination and streaming #70

Merged
merged 8 commits into from
Mar 30, 2016
Merged

Conversation

jeffweiss
Copy link
Contributor

Close #67

@coveralls
Copy link

Coverage Status

Coverage increased (+2.4%) to 86.702% when pulling c57578d on jeffweiss:add_pagination into f954f84 on edgurgel:master.

@jeffweiss
Copy link
Contributor Author

Hmm.. I'm using URI.to_string, which is only available in Elixir 1.1+. @edgurgel, @duksis, thoughts on bumping minimum Elixir version to 1.1?

@jeffweiss
Copy link
Contributor Author

Also just saw #69 merged, I should update to use those options as well. Standby...

@coveralls
Copy link

Coverage Status

Coverage increased (+2.4%) to 86.702% when pulling 8de890f on jeffweiss:add_pagination into f954f84 on edgurgel:master.

@duksis
Copy link
Collaborator

duksis commented Mar 29, 2016

first of all - thanks for the amazing contribution @jeffweiss !

In regards of bumping required Elixir version, I would prefer backporting URI.to_string for Elixir 1.0 than dropping the support for it.
What's your take on this @edgurgel ?

@edgurgel
Copy link
Owner

I agree, @duksis. Let's do this in 2 stages.

@jeffweiss
Copy link
Contributor Author

Oh, we might not need to backport anything. It looks like String.Chars.to_string(URI) was defined in 1.0 even though URI.to_string was not. This might be a very simple change.

Prior to this commit Tentacat was using `URI.to_string` for converstion
of the URI struct to a binary. While this works in Elixir 1.1+, Elixir
1.0 does not have a `URI.to_string`. This commit changes the function
call to `String.Chars.to_string(%URI{})` which *does* exist in Elixir
1.0 (and later).

NOTE: I expect this commit to be reverted for readability once Elixir
1.0 support is dropped.
@coveralls
Copy link

Coverage Status

Coverage increased (+2.4%) to 86.702% when pulling b6c9c55 on jeffweiss:add_pagination into f954f84 on edgurgel:master.

@jeffweiss
Copy link
Contributor Author

\o/ --- If you have other changes you'd like me to make lemme know.

@duksis
Copy link
Collaborator

duksis commented Mar 30, 2016

thanks! We will let you know when other changes will be needed. 😉

@duksis duksis merged commit 223d306 into edgurgel:master Mar 30, 2016
@jeffweiss jeffweiss deleted the add_pagination branch March 30, 2016 07:55
@edgurgel
Copy link
Owner

👍 We should release a new version with this contribution. I should give owner access to you @duksis. Do you have an e-mail registered with hex so I can add you? :)

@duksis
Copy link
Collaborator

duksis commented Mar 30, 2016

@edgurgel should be duksis@gmail.com

@edgurgel
Copy link
Owner

$ mix hex.owner add tentacat duksis@gmail.com
Adding owner duksis@gmail.com to tentacat

🎉

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants