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 support for Umbrella-app scoped configuration #53

Closed
wants to merge 35 commits into from

Conversation

feda12
Copy link

@feda12 feda12 commented May 13, 2019

Currently, it is not possible to define multiple configurations in an umbrella app because the configuration variables are global.

This addresses this by attempting to find an app-specific config object first, then defaulting to a global one. This ensures backward compatibility.

@ghost
Copy link

ghost commented May 17, 2019

👍
I have only one phoenix app in my umbrella and having an issue because of the https://github.com/KittyHeaven/blue_bird/blob/master/lib/blue_bird/generator.ex#L88

My umbrella will run the different test and the last one that runs is not the phoenix app. So Project.get().application will not return the right app, if this app doesn't have a :mod it will crash.

Seems like this PR is fixing my issue because I am happy with the default configuration.

But I think you will have unpredictable results with

    Project.get().project()
    |> Keyword.get(:app)
    |> Application.get_env(:blue_bird, nil)

because of the order of the tests.

@feda12
Copy link
Author

feda12 commented May 21, 2019

Sorry for the delay, been attempting to work around it. Will update when I have something solid.

@feda12
Copy link
Author

feda12 commented May 24, 2019

I made more changes to accommodate our own needs. If you think these make sense, happy to merge them otherwise I am going to close this for now.

@feda12 feda12 closed this May 24, 2019
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

1 participant