Covering all patterns and preventing errors

[sanposhiho]

Hi :)
fix auth.ex to cover all patterns and preventing errors

Including this change will give you peace of mind if we get an unintended response :)

For Example
httpc.request may return {:error, hoge}, and down application using extwitter, because you intended only httpc.request return {:ok, hogehoge}
http://erlang.org/doc/man/httpc.html

[coveralls]

Coverage increased (+0.04%) to 81.89% when pulling 17b543a on sanposhiho:fix-baaad-error-handling into d3a768c on parroty:master.

[parroty]

Thanks for the PR!

It seems one of the test is failing. Corresponding test code can be updated for this change?

• https://travis-ci.org/github/parroty/extwitter/builds/739200024

[sanposhiho]

fix conflict and test case :)

[parroty]

Thanks for the fix.
The test code change is coming from the code formatter?
Indentation change may be ok, but I think ID format is just a sequence of digits rather than numeric value, and I don't think splitting by 3 digits makes much sense.

[sanposhiho]

The test code change is coming from the code formatter?

Yes!

I don't think splitting by 3 digits makes much sense.

I agree with you; 444144169058308096 is a number that represents an id that is not particularly meaningful here, and readability is not required here.

But the important fact here is not whether this number conversion is meaningful or not, but rather that the official Elixir formatter made the change to this code.

Almost all good Elixir developers include a setting that uses formatting all the time (with settings of editor). It's also one of the nice things about Elixir that there is an official formatter and that everyone uses the same formatter.

The PRs from those developers will contain the same diffs every time, so I'd prefer to keep the numbers with _ in them

(If you still want to roll back these changes, I'll roll back these changes :) )

[parroty]

Thanks for the description, and I understood that putting special treatment make it cumbersome.

[chevinbrown]

Did this change the response of access_token from {:ok, token} to {:ok, {:ok, token}}?

https://github.com/parroty/extwitter/pull/133/files#diff-9f19231ea6327cd67d05e4e47d306650f410158fd78ddd2c050a548d4bdc81c7L569

Looks like this is a breaking change.

[parroty]

Ops, thanks for the report. I wasn't intending to change the existing behavior. Maybe I should revert this change and re-introduce in different manner?

[parroty]

Sorry, but please let me revert at first (though the original return-value I/F and error handling may not be good), I wasn't thinking enough about the change in test-code part.

[sanposhiho]

Did this change the response of access_token from {:ok, token} to {:ok, {:ok, token}}?

I change the behaviour from token to {:ok, token} in this PR related to #67
https://github.com/parroty/extwitter/pull/133/files#diff-301233dd8df072574b8dfdb164c3423f8a32e1f3df5b10562328775e0f04f548R113-R114

Maybe I should have split the PR, sorry :'-(

[chevinbrown]

@sanposhiho @parroty it's ok, but it would definitely help to be more clear in the changelog & versioning. Thanks for your quick work on this!