Create a bare project by default (no supervisor) #2074

Closed
devinus opened this Issue Feb 25, 2014 · 38 comments

Comments

Projects
None yet
Contributor

devinus commented Feb 25, 2014

A lot of Alchemists today (even experienced ones) seem confused about when they should keep the supervisor mix new creates for them. I believe we should make people think about when they need a real application in the first place versus making them question if they should remove it.

Examples of new apps that should be library apps but include the default (useless) supervisor and start themselves as real applications:

https://github.com/phoenixframework/phoenix/blob/master/lib/phoenix/supervisor.ex
https://github.com/phoenixframework/ex_conf/blob/master/lib/ex_conf/supervisor.ex
https://github.com/nurugger07/inflex/blob/master/lib/inflex/supervisor.ex
https://github.com/knewter/exdrone/blob/master/lib/exdrone/supervisor.ex

Taken from a random smattering of https://github.com/search?l=elixir&p=1&q=stars%3A%3E1&s=updated&type=Repositories, sorry I can't PR everybody.

@chrismccord @nurugger07 @knewter

Member

chrismccord commented Feb 26, 2014

This is a good discussion. In Phoenix's case, I wasn't sure which route to go supervisor wise, but I agree ExConf should not have the useless superviser. I left it out of carelessness after mix new. @devinus What ideas do you have for making people think about real applications vs libs? I think docs highlighting each case might be all that's necessary. Otherwise, we could require something like mix new.app mix new.lib instead of mix new vs mix new --bare?

Contributor

devinus commented Feb 26, 2014

@chrismccord I'm thinking we have mix new (creates a library app, the default), and mix new --supervisor (has the application callback in mix.exs and starts a supervisor, the functionality mix new has today).

A cursory glance at a lot of popular Elixir projects in that link leads me to believe that a majority of the new projects should be library apps but a majority of those are starting the useless supervisor. (I wish I had the time to PR all of them, but I just saw way too many.)

@ericmj ericmj added App:Mix and removed Kind:Enhacement labels Feb 26, 2014

👍

Member

fishcakez commented May 3, 2014

👎 The ratio on github is not representative:

  • Elixir is relatively new and so prone to having people race to write library applications that already exist in every other language (like json encoders).
  • Elixir is relatively new so open source projects there is a higher ratio of relatively basic projects for now.
  • Library applications are more likely to be open sourced due to there more general purpose - and one needs these to write more complex ones.
  • It is faster to delete in the cases where it's not needed than it is to write it where it is.
Member

alco commented May 3, 2014

It is faster to delete in the cases where it's not needed than it is to write it where it is.

True, but it'd be annoying to do this every time.

What about adding another required argument to mix new?

  • mix new app <path> -- creates an application template with supervisor (default today)
  • mix new lib <path> -- create a library template without applications in mix.exs

This will be helpful both for beginners and adepts. Even if a beginner doesn't make the right choice at first, they will at least try to learn the difference.

It's like using Go-style error ignoring (val, _ = some_func()) vs explicitly handling the error part of the option type (case some_func() do {:ok, value} -> value; :error -> ...). If you choose the first one (the current behaviour of mix), you'll inevitably get libraries with useless supervisors (or missing supervisors if the default is flipped).

Owner

josevalim commented May 3, 2014

That's an interesting option @alco. Should it be new app and new lib or new.app and new.lib?

Member

fishcakez commented May 3, 2014

I think lib/app is a bad choice because they are both applications and anything that suggests otherwise is misleading.

Owner

josevalim commented May 3, 2014

Maybe the names are bad but the point is about forcing the users to make a decision which one they won't instead of picking one as default.

Contributor

matheusml commented May 9, 2014

What about keeping the old mix new, which creates a default project,
but adding the options that @alco said.

Something like:

    mix new my_project
    mix new app my_project
    mix new lib my_project
    mix new webapi my_project
Contributor

bitwalker commented May 9, 2014

I like @matheusml's suggestion, since it mirrors lein quite nicely with how it handles custom project templates. One could imagine a mix new phoenix my_project command which would generate a blank phoenix project.

I do think using lib to refer to bare apps is a bad idea, but we talked that to death in IRC the other day.

Contributor

knewter commented May 9, 2014

I like @matheusml's suggestoin as well, and especially if it would include custom templating later :)

Owner

josevalim commented May 9, 2014

@matheusml this idea is interesting however we would need to re-invent what mix already does. For example, how do I get the help for mix new app? How do I implement a new template? Maybe we should just advocate for a mix new.app/mix new.phoenix style?

Member

alco commented May 9, 2014

Agree with @bitwalker: add support for arbitrary templates in mix new.

There could be templates shipping with mix, one with a supervisor, another one without, etc.

As for which template to use by default, there are still multiple approaches:

  1. require the user to pass a template as an argument, so mix new won't work.
  2. organize a poll to decide on the default template :)
Contributor

knewter commented May 9, 2014

I would like mix new.app just as much. Seems more consistent with
existing task patterns as well.

Owner

josevalim commented May 9, 2014

And we could have mix new simply listing all existing mix new.*.

Contributor

bitwalker commented May 9, 2014

That seems reasonable, and would make plugging in to the existing architecture a whole lot easier.

Member

alco commented May 9, 2014

I think that's limiting. In the future you could have a lot of new.* in the mix help output which is not what I call convenient. Also, you have to install a template locally to be able to use it.

Adding support for arbitrary templates means the template itself can come from any source, including URL, without the need to preinstall or keep it on your system.

Contributor

bitwalker commented May 9, 2014

I'd need to look at how lein works, but they fetch templates on demand which I think is a killer feature. Perhaps Mix could be extended to work similarly?

Contributor

MSch commented May 9, 2014

I like @matheusml suggestion too, but if it's (too) much work I vote for keeping the supervisor by default, because the supervisor is always needed when writing a "real" application.

I agree with @fishcakez about how and why we're seeing a proliferation of libraries on GitHub, and I think the overwhelming number of projects created (not open sourced) will always be "apps" not "libraries" and those all need a supervisor.

Owner

josevalim commented May 9, 2014

@alco I would say that is also reinventing mix functionality. We already have mix local.install that installs and works on packages. For me such approach is preferable than downloading, compiling and executing remote code directly.

Contributor

matheusml commented May 9, 2014

+1 to mix new.*
I can live with that.

Contributor

bitwalker commented May 9, 2014

Here's how lein does it now (https://github.com/technomancy/leiningen/blob/master/src/leiningen/new.clj). After looking at it, I agree with @josevalim that using mix local.install and having mix new.* probably makes the most sense. We could probably extend mix new.<whatever> to first check if the package is installed locally, and if not, check hex.pm for a package of that name, and install it if it's available. That would provide almost the same exact functionality that lein new some_app my_project does now.

Member

alco commented May 9, 2014

@josevalim it doesn't need to execute code. A template is just a directory structure with some placeholders replaced by user-supplied values. Can be implemented in a very basic manner: by string substitution.

Going the mix local.install <url>; mix new.something route is a hassle. I already fear what my mix help output will look like a year from now.

I just don't see why I would need to install phoenix, to then invoke mix new.phoenix so that it downloads phoenix again as a dependency. I would much more prefer for mix new to accept 3rd party templates. Then phoenix can build a template that includes phoenix-related mix tasks in the generated project.

This will enable phoenix tasks only inside a phoenix project's directory. So, if I have 5 projects each using different set of custom mix tasks, it'd be more manageable to have those tasks show up only within each project's dir as opposed to being installed system-wide.

Sorry for overusing phoenix as an example.

Owner

josevalim commented May 9, 2014

If we are assuming templates are just a bunch of files. Then yes, there is benefit in using templates in Mix. However, how far does it work? If you have tons of logic, is it basically shoved in the template? What if I want custom command line args?

Contributor

bitwalker commented May 9, 2014

@alco, lein works by installing the dependency locally and then looking for the template in a standard location, mix would have to work the same way to enable mix new phoenix my_app. Otherwise how else would it know where to get the template?

Contributor

bitwalker commented May 9, 2014

@josevalim what are your thoughts about my suggestion around mix new.whatever looking to see if the whatever package has been installed locally, and if not, fetching it from hex.pm if it's available? If it's not available, then a sensible error is reported, likewise if the package is available, but there is no new task for the package.

Then projects could implement their own templates via a task like new.whatever which could also accept command line args for options.

Member

alco commented May 9, 2014

@josevalim If you need custom logic in the process of creating a project from a template, you will indeed need to use a mix task for that. But I don't think it should be common. Making mix new accept simple template by default seems more convenient to me that forcing all templates to go the local.install route.

@bitwalker mix new https://gist.gihub.com/my_cool_template proj_name

Owner

josevalim commented May 9, 2014

That's an interesting idea although hex doesn't work with standalone packages like that just yet.

Contributor

bitwalker commented May 9, 2014

@alco that's not a bad way to go, but I think template options are going to be more common than you might think. Consider all the options Rails has for creating a new project, Phoenix may want the same kind of flexibility, and I could see it be relatively common to want to provide flags for features in your template. Something to think carefully about anyways, before making a decision one way or the other.

Member

fishcakez commented May 9, 2014

I agree with @MSch, supervisor by default. Even if you don't start out requiring a supervisor you might need one later when the project evolves.

I really dislike differentiating between libraries and applications. Both are applications. A library is still an application, every mix project is an application. That is why I was against the lib versus app option, I would rather use different terminology.

Contributor

sasa1977 commented May 9, 2014

For me, a library is a shorthand for library application. That's how it's called in Erlang, so I don't see anything wrong with it.

Personally, I dislike mix generating supervisor by default. I'm in favour of mix new.* without a default. This should among other things, force people to learn the difference early on.

To please @fishcakez we could have new.app and new.lib_app. This might provide a stronger indication that both are apps. Personally, I'm fine with just lib.

Contributor

devinus commented May 9, 2014

@fishcakez

Even if you don't start out requiring a supervisor you might need one later when the project evolves.

Well, that's precisely the problem that I'd like addressed in this issue. People are leaving that stuff in there in all of these libraries, running application callbacks and starting supervisors that don't need to be there.

Owner

josevalim commented May 10, 2014

Here is a related discussion. v0.13.2 will introduce configuration files. Should those files be generated with the application or should we have a mix new.config task that generates it? Here is the sample file:

embed_template :config, ~S"""
# This file is responsible for configuring your application and
# its dependencies. It must return a keyword list containing the
# application name and have as value another keyword list with
# the application key-value pairs.
# Note this configuration is loaded before any dependency and is
# restricted to this project. If another project depends on this
# project, this file won't be loaded nor affect the parent project.
# You can customize the configuration path by setting :config_path
# in your mix.exs file. For example, you can emulate configuration
# per environment by setting:
#
# config_path: "config/#{Mix.env}.exs"
#
# Changing any file inside the config directory causes the whole
# project to be recompiled.
# Sample configuration:
#
# [dep1: [key: :value],
# dep2: [key: :value]]
[]
"""

Member

fishcakez commented May 10, 2014

It's when the implication (even if it is not meant) is that a decision is being made between a library OR an application that I dislike because a library is an application. I am not against describing an application as a library when it doesn't have a supervision tree. If mix new.* style is used I think my point maybe moot because all but umbrella templates are going to be applications anyway, though I see no harm in mix new.app --bare (matching how it works currently).

Not using the --bare option in the current mix new is not a mistake even if the application could be a library. It does not matter if an application has an empty supervisor. There is no problem with it and no harm is done if an application that could be a library is not one. However if a supervisor is required, which it usually is, then a supervisor and application behaviour is needed and so a default without a supervisor would be a bad decision in my opinion. Obviously if there is no longer a default this does not matter.

Contributor

sasa1977 commented May 10, 2014

Personally, I'm more bothered by an implication of a generator including something I don't want. Putting my beginner's hat on, if I see mix new generating supervisor, I'll assume it's something that needs to be here. On the other hand, if I try to start the app and nothing happens, I'll research what must be done. This allows me to gradually learn and experiment, while at the same time maintaining proper code structure. I could develop a properly structured json parser app lib without even knowing what supervisors, gen_servers, or processes are.

Given Devin's clear demonstration that even prominent members from the community have supervisors in app libs, it's fair to expect this will spread as more people flock to Elixir and start creating simple libs. And the main reason for this is in the tool, which includes supervisor by default.

Hence, I prefer either no supervisor by default, or explicit decision between app and a lib (via mix new.*). The more I think about it, the more I prefer the latter approach.

Contributor

bitwalker commented May 10, 2014

I like the idea of generalizing templates, bundling mix new.app and mix new.lib, and using the help documentation for each to talk about their use cases. Something along the lines of "mix new.app is used for applications which require supervision, if your app does not require this, mix new.lib is the best choice for your project, if you aren't sure whether you need supervision, see the docs at ". This makes it clear which one you should use and when, but offers new users a chance to find out more if they have no idea which choice to make. This also means that they'll have to learn about supervision at a high level relatively early in order to make the choice, but I see that as a good thing. As part of this I think we'd want to get rid of mix new someapp, and make it so you always have to specify the template, i.e mix new.lib somelib, mix new.app someapp, mix new.phoenix somewebapp, etc.

Contributor

smpallen99 commented May 12, 2014

I like the way mix new works today. In most cases, I want the supervisor. As well, I think its a good reminder to newbies too. Adding an option to create a lib would be nice, but I again, I would prefer that the existing mix new not change. It does not really bother me to delete the supervisor if I don't want it.

Owner

josevalim commented May 26, 2014

Thanks everyone. There has been good arguments on both sides, which makes it hard to choose a particular solution, however I did choose to make the proposed change. From Elixir v0.14.0 onwards:

  1. mix new will generate an application without a supervision tree by default
  2. --sup must be given in order to get a supervision tree

In both cases a config/config.exs will be generated (which is local and does not affect your dependencies).

@josevalim josevalim closed this in 43706a4 May 26, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment