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

bootleg.invoke accepts deploy env as first argument #222

Merged
merged 5 commits into from
Feb 19, 2018

Conversation

rjanja
Copy link
Contributor

@rjanja rjanja commented Feb 18, 2018

Closes #221

@rjanja rjanja requested a review from brienw February 18, 2018 19:40
@@ -9,14 +9,22 @@ defmodule Bootleg.MixTask do
@spec run(OptionParser.argv) :: :ok
if is_atom(unquote(task)) && unquote(task) do
def run(args) do
Config.env(List.first(args) || :production)
Copy link
Contributor Author

@rjanja rjanja Feb 18, 2018

Choose a reason for hiding this comment

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

We already set a default production enviroment in Config.env/0 so the setting of defaults here is unnecessary I think.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, agreed. in fact, when i was running into the "environment config not set" error last week in our test suite after paul's IO refactoring it actually took me a bit to find this default that was causing it because it's so buried.

# end
# ```
# For more about `bootleg_phoenix` see: https://github.com/labzero/bootleg_phoenix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's better to not include sample dependencies than to potentially include outdated versions by accident

{:bootleg_phoenix, "~> 0.2"}]
end
```

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't need to include sample deps for additional packages, when doing so it just makes it harder to update versions

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@brienw brienw left a comment

Choose a reason for hiding this comment

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

LGTM

{:bootleg_phoenix, "~> 0.2"}]
end
```

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -9,14 +9,22 @@ defmodule Bootleg.MixTask do
@spec run(OptionParser.argv) :: :ok
if is_atom(unquote(task)) && unquote(task) do
def run(args) do
Config.env(List.first(args) || :production)
Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, agreed. in fact, when i was running into the "environment config not set" error last week in our test suite after paul's IO refactoring it actually took me a bit to find this default that was causing it because it's so buried.

@brienw brienw merged commit 5c797a2 into labzero:master Feb 19, 2018
brienw pushed a commit that referenced this pull request Mar 1, 2018
* Invoke accepts bootleg env as first argument

* Refactor and consolidate references to config paths

* More refactoring

* Add bootleg.invoke to README

* One more refactor
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.

2 participants