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

add uri tests #48

Merged
merged 1 commit into from
May 13, 2016
Merged

add uri tests #48

merged 1 commit into from
May 13, 2016

Conversation

stavro
Copy link
Contributor

@stavro stavro commented May 13, 2016

After starting to add some tests, I realized that the current URI implementation is very broken. Some of these tests are still failing.

I strongly suggest deprecating the usage of keyword lists as arguments, and only use maps.

_Don't do this_:

  card = [
    card: [
      number: 424242424242,
      exp_year: 2014
    ]
  ]

_But, do this:_

  card = %{
    card: %{
      number: 424242424242,
      exp_year: 2014
    }
  }

Then you can delegate all of the query string formatting directly to Elixir's URI.encode_query implementation, which will handle it properly.

@coveralls
Copy link

Coverage Status

Coverage increased (+5.07%) to 18.478% when pulling 5b979eb on stavro:master into 29c0076 on robconery:master.

@robconery
Copy link
Contributor

Love it. I forked this from a dead repo and have been meaning to update the codebase - appreciate your help here :).

@robconery robconery merged commit 0e4d31b into beam-community:master May 13, 2016
@stavro
Copy link
Contributor Author

stavro commented May 13, 2016

np!

If you want help trying to update some of this code and come up with a proper upgrade path let me know. I'm also on the elixir irc chatroom if you wanna talk about it.

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