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

Set json library on OAuth2 #58

Merged
merged 7 commits into from
Mar 15, 2019

Conversation

snewcomer
Copy link
Contributor

@snewcomer snewcomer commented Jan 5, 2019

@yordis
Copy link
Member

yordis commented Jan 6, 2019

@snewcomer Could you use https://github.com/ueberauth/ueberauth/blob/2087e031c3ff486e0aafb4684659a3e224d2aa29/lib/ueberauth.ex#L190 instead?

@mspanc
Copy link
Contributor

mspanc commented Mar 15, 2019

Hi, any chance to release this soon? I can help if necessary.

Currently if you use the most recent versions of ueberauth and ueberauth_google they do not work well togehter.

ueberauth will try to use Jason

iex(2)> Ueberauth.json_library
Jason

but ueberauth_google will try to use Poison

ueberauth 0.6.1
ueberauth_google 88b9ae8

@snewcomer
Copy link
Contributor Author

Ah I'll update right now!

@snewcomer snewcomer force-pushed the sn/oauth-register-serializer branch from 8b45a14 to 6c1b383 Compare March 15, 2019 14:34
@snewcomer snewcomer changed the title WIP Set json library on OAuth2 Set json library on OAuth2 Mar 15, 2019
@snewcomer
Copy link
Contributor Author

@mspanc Updated. I was searching around for quite a while since I knew there was another PR somewhere. Thx for helping dig it out!

This reverts commit 381bc29.
@mspanc
Copy link
Contributor

mspanc commented Mar 15, 2019

@snewcomer awesome, thanks!

@yordis any chance for merging/releasing?

@yordis
Copy link
Member

yordis commented Mar 15, 2019

@mspanc I am confused, the PR does not use Ueberauth.json_library unless I am missing something

Copy link
Contributor

@mspanc mspanc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way you query for the Ueberauth's JSON library is invalid, IMO

@@ -27,7 +27,10 @@ defmodule Ueberauth.Strategy.Google.OAuth do
def client(opts \\ []) do
config = Application.get_env(:ueberauth, __MODULE__, [])
opts = @defaults |> Keyword.merge(opts) |> Keyword.merge(config) |> resolve_values()
json_library = Application.get_env(:ueberauth, :json_library)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@snewcomer

shouldn't that be

json_library = Ueberauth.json_library()

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

glad somebody looked at my code 😆

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@snewcomer 😆

@yordis yordis merged commit 0aa46b1 into ueberauth:master Mar 15, 2019
@yordis
Copy link
Member

yordis commented Mar 15, 2019

@snewcomer 🚀

@snewcomer snewcomer deleted the sn/oauth-register-serializer branch March 15, 2019 20:55
@doomspork
Copy link
Member

@yordis are we ready to cut a new release? If so, will you prep the PR to do so?

@yordis
Copy link
Member

yordis commented Mar 15, 2019

@doomspork I want to start adding all the for GitHub like such https://github.com/straw-hat-team/straw_hat/tree/master/.github

I just need to add the checkbox in Pull Request asking people to add a log to the CHANGELOG file (I need to update Straw Hat as well)

I will be working on this today for sure.

@rschooley
Copy link

Could we get a release with this update? v0.8.0 breaks on the hard poison dependency.
Thanks for fixing 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.

5 participants