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 new API #83

Merged
merged 2 commits into from
Jan 27, 2021
Merged

Add new API #83

merged 2 commits into from
Jan 27, 2021

Conversation

wojtekmach
Copy link
Collaborator

@wojtekmach wojtekmach commented Jan 11, 2021

Ref: #82

This is extracted from https://github.com/wojtekmach/goth, a fork we've created at Dashbit a while ago.

Also:

* Move application callback module to Goth.Application.
@lytedev
Copy link
Contributor

lytedev commented Jan 14, 2021

Thanks for linking this fork! It's looking like this upstream is pretty inactive. Perhaps we can move this repo to the Dashbit one? I'm not sure exactly what all that entails, but I imagine it will take some cooperation from both you and @peburrows?

@wojtekmach
Copy link
Collaborator Author

@peburrows invited me to help maintaining this repo and I intend to upstream all the changes from my fork and release them as goth v1.3.0 on Hex so stay tuned!

Copy link
Owner

@peburrows peburrows left a comment

Choose a reason for hiding this comment

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

I have no major feedback that would change the core approach here — looks great. I'm so glad someone else cares enough about this little library to give it the attention I haven't had the time to over the last year or so!

```elixir
# config/config.exs
config :goth,
json: {:system, "GCP_CREDENTIALS"}

Choose a reason for hiding this comment

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

what about the idea; instead of passing the variable name, it could digest pure value and let allow the client to configure System.fetch_env!/1 on hiw own

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

note this is an attempt at an upgrade guide. People should be moving away from code like this to what is mentioned below. :)

Choose a reason for hiding this comment

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

ah, you're right! then, on the same page, thanks

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