Automatically inflect the list of applications, closes #5249 #5473

josevalim · 2016-11-21T09:56:57Z

Before merging this, we need to validate two design decisions:

The list of applications is built per environment. This means test only dependencies, such as {:meck, "> 0.0.0", only: :test}, will be added to the list of applications when compiled for test (and only for test). This means :meck no longer needs to be started manually.
To disable a dependency from being added to the list of applications, we can pass the runtime: false to the dependency. Such as {:mech, "> 0.0.0", only: :test, runtime: false}. This will disable it from being added to any environment, specially test in this case. Is :runtime the best name here?

The reason I decided to go with 1. is because inflecting the list of dependencies becomes hard otherwise. For example, if we decide to only include the list of production dependencies, what would happen if a dependency is runs only in production? Then the application would never boot for dev and test. Then you could say: well, maybe we should add only production dependencies that also belong to the current environment. Which means we would have per-environment behaviour anyway so I decided to go all the way by including all deps in the current environment because it is simpler (simpler to explain, understand and code).

Thoughts?

whatyouhide · 2016-11-21T10:04:02Z

About the :runtime name, I'm not sure I'm a fan (yet 😛). It has to convey that the application is not added to the list of applications, and maybe something like in_applications: true | false could make sense? Not sure.

Also, I have a question: what happens if I want to use :included_applications? That is, I want an app to end up in my release but I want to start it manually?

Edit: I also left a few comments regarding formatting and other silly things in the documentation :)

whatyouhide · 2016-11-21T10:04:23Z

lib/mix/lib/mix/tasks/compile.app.ex

Missing final period.

The other items on the list do not have a final period.

whatyouhide · 2016-11-21T10:07:03Z

lib/mix/test/mix/tasks/compile.app_test.exs

Should we add a test here for validation of :extra_applications as well? We have it for all other options, it may be weird to skip this one. 😕

whatyouhide · 2016-11-21T10:07:35Z

lib/mix/lib/mix/tasks/deps.ex

Instead of "if the dependency ...", we could have "whether the dependency ...".

Say what about the weather?

whatyouhide · 2016-11-21T10:08:00Z

lib/mix/lib/mix/tasks/deps.ex

Backticks around true :)

josevalim · 2016-11-21T10:08:11Z

something like in_applications: true | false could make sense?

The reason I am not a fan of in_applications: true is because it is the indirect behaviour. The reason you add an application or not to :applications is if you need it at runtime or not. So it feels the option should reflect the final behaviour and not the intermediate one.

Good call about :included_applications, I will address that.

Closes #5249

whatyouhide · 2016-11-21T10:13:36Z

lib/mix/lib/mix/tasks/compile.app.ex

The other items on the list do not have a final period.

Then no final period here 😄

On this list there is a final period on the items. I am just keeping it consistent with the surrounding lists (although Elixir is not consistent).

Ah, alrighty sorry for the noise then!

fishcakez · 2016-11-21T13:25:32Z

I don't like :runtime because many of OTP's apps have a :runtime_dependencies key, which might become confusing if/when it becomes more documented/adopted. Why not start: true | false?

michalmuskala · 2016-11-21T13:38:56Z

I like the start suggestion. The start: true/false could be extended to start: :permanent/transient/temporary for even more control.

josevalim · 2016-11-21T13:49:49Z

:start it is.

fishcakez · 2016-11-21T13:49:54Z

@michalmuskala how would we control the start type of project or extra applications? If we want to go this route then we'll need to build on proposal.

josevalim · 2016-11-21T13:52:12Z

Actually, :start is not good because you can have an application that you want included in a release but not started. However, this option is doing both: it is not including it in the release and not starting it.

josevalim · 2016-11-21T14:20:41Z

❤️ 💚 💙 💛 💜

whatyouhide · 2016-11-21T14:21:35Z

This is awesome, great work @josevalim!

fishcakez · 2016-11-21T14:22:42Z

I don't like :runtime because many of OTP's apps have a :runtime_dependencies key, which might become confusing if/when it becomes more documented/adopted

As discussed on irc even if this did become mainstream it would not be confusing.

ericmj · 2016-11-21T14:29:10Z

lib/mix/lib/mix/tasks/compile.app.ex

      {:applications, value} ->
        unless is_list(value) and Enum.all?(value, &is_atom(&1)) do
-          Mix.raise "Application dependencies (:applications) should be a list of atoms, got: #{inspect value}"
+          Mix.raise "Application applications (:applications) should be a list of atoms, got: #{inspect value}"


"Application applications"?

And above "Application extra applications".

Since elixir-lang/elixir#5473 in Elixir 1.4, the `applications` key is automatically populated by Mix based on `deps` - but ONLY if it's not manually specified in `mix.exs`. Adding an explicit `applications` key leads to user confusion, since other dependencies that were expecting to be automatically started won't be. Also updates the example version to match the latest release.

josevalim mentioned this pull request Nov 21, 2016

Automatically inflect the list of applications #5249

Closed

whatyouhide reviewed Nov 21, 2016

View reviewed changes

josevalim force-pushed the jv-app-inflect branch from 4dec71c to 3a11a9c Compare November 21, 2016 10:11

Automatically inflect the list of applications

50478ae

Closes #5249

whatyouhide reviewed Nov 21, 2016

View reviewed changes

josevalim force-pushed the jv-app-inflect branch 2 times, most recently from d3a3e4b to 50478ae Compare November 21, 2016 10:54

josevalim merged commit 75e2ee0 into master Nov 21, 2016

josevalim deleted the jv-app-inflect branch November 21, 2016 14:21

ericmj reviewed Nov 21, 2016

View reviewed changes

bitwalker mentioned this pull request Jul 13, 2017

Expand the capability of the :runtime option in Mix #6318

Closed

bradleygolden mentioned this pull request Aug 19, 2020

Include runtime dependencies c-rack/plug_cloudflare#16

Merged

al2o3cr mentioned this pull request Mar 7, 2023

Update README for modern Mix practices maennchen/email_checker#21

Merged

Automatically inflect the list of applications, closes #5249 #5473

Automatically inflect the list of applications, closes #5249 #5473

Uh oh!

Conversation

josevalim commented Nov 21, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

whatyouhide commented Nov 21, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josevalim commented Nov 21, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fishcakez commented Nov 21, 2016

Uh oh!

michalmuskala commented Nov 21, 2016

Uh oh!

josevalim commented Nov 21, 2016

Uh oh!

fishcakez commented Nov 21, 2016

Uh oh!

josevalim commented Nov 21, 2016

Uh oh!

josevalim commented Nov 21, 2016

Uh oh!

whatyouhide commented Nov 21, 2016

Uh oh!

fishcakez commented Nov 21, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

6 participants

josevalim commented Nov 21, 2016 •

edited

Loading

whatyouhide commented Nov 21, 2016 •

edited

Loading