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

Use @preferred_cli_env #96

Merged
merged 1 commit into from
Mar 20, 2017
Merged

Use @preferred_cli_env #96

merged 1 commit into from
Mar 20, 2017

Conversation

josevalim
Copy link
Contributor

Elixir provides the ability to use @preferred_cli_env directly on the task since version v1.3.0. This PR declares those attributes by default.

Keep in mind that:

  1. This won't work for Elixir versions earlier than v1.3.0. If you still support them, you may want to add the line about preferred_cli_env back to the README

  2. The README will be outdated as soon as this is merged, so you may want to consider a new release

Thank you! ❤️

@coveralls
Copy link

coveralls commented Mar 17, 2017

Coverage Status

Coverage remained the same at 92.26% when pulling e5e00f7 on josevalim:master into 9721961 on parroty:master.

@joshsmith
Copy link
Collaborator

Thanks @josevalim! ❤️ 💚 💜 💛 💙

@parroty LGTM whenever you have a chance to cut a new release.

@parroty parroty merged commit f7c9963 into parroty:master Mar 20, 2017
@parroty
Copy link
Owner

parroty commented Mar 20, 2017

Thanks, I wasn't aware of the feature, and will be updating the release ❤️.

@parroty
Copy link
Owner

parroty commented Mar 31, 2017

Hi @josevalim.
I'm trying to apply this preferred_cli_env to other locations, but facing some errors. I'm wondering about what the good practice definition is like.
(I think I'm misunderstanding something but any helps are appreciated. Also, I'm sorry about spreading around bad practices 😓.)

1. Test dependency

If I try to remove the definition from excoveralls' mix.exs like 497b2f6 , module Mock is not loaded is occuring.

It seems modified preferred_cli_env allows to identify mix coveralls task (without MIX_ENV=test, but it seems test only dependencies are not loading (MIX_ENV=test mix coveralls works). How they should be defined?

$ mix coveralls
** (CompileError) test/circle_test.exs:3: module Mock is not loaded and could not be found
    (stdlib) erl_eval.erl:670: :erl_eval.do_apply/6
    (elixir) lib/code.ex:370: Code.require_file/2
    (elixir) lib/kernel/parallel_require.ex:57: anonymous fn/2 in Kernel.ParallelRequire.spawn_requires/5
$ MIX_ENV=test mix coveralls
----------------
COV    FILE                                        LINES RELEVANT   MISSED
 44.4% lib/excoveralls.ex                            105       18       10
 93.3% lib/excoveralls/circle.ex                      72       15        1
 88.9% lib/excoveralls/conf_server.ex                 44        9        1
 50.0% lib/excoveralls/cover.ex                       43       10        5
100.0% lib/excoveralls/exceptions.ex                  11        0        0
100.0% lib/excoveralls/html.ex                        43       12        0
100.0% lib/excoveralls/html/safe.ex                   28        2        0
 90.0% lib/excoveralls/html/view.ex                   44       10        1
100.0% lib/excoveralls/json.ex                        39       10        0
100.0% lib/excoveralls/local.ex                      148       49        0
100.0% lib/excoveralls/path_reader.ex                 20        2        0
 80.0% lib/excoveralls/post.ex                        33        5        1
 85.7% lib/excoveralls/poster.ex                      47       14        2
 92.9% lib/excoveralls/semaphore.ex                   67       14        1
100.0% lib/excoveralls/settings.ex                    89       19        0
100.0% lib/excoveralls/stat_server.ex                 21        4        0
100.0% lib/excoveralls/stats.ex                      223       60        0
100.0% lib/excoveralls/stop_words.ex                  37        7        0
100.0% lib/excoveralls/sub_apps.ex                    21        5        0
100.0% lib/excoveralls/task/util.ex                   43        1        0
 88.9% lib/excoveralls/travis.ex                      39        9        1
 95.8% lib/mix/tasks.ex                              215       48        2
[TOTAL]  92.3%

2. Loading mix tasks from test dependency

I' trying to load updated library from other project like the following, but fails to identify mix tasks.

It's defined as {:excoveralls, "~> 0.6", only: :test},, but this should be modified too? (ex. make it as only: [:dev, :test] works though).

➜  extwitter git:(fix/preferred_cli_env) mix coveralls
** (Mix) The task "coveralls" could not be found

Current environment is

$ elixir -v
Erlang/OTP 19 [erts-8.2] [source] [64-bit] [smp:4:4] [async-threads:10] [hipe] [kernel-poll:false] [dtrace]

Elixir 1.4.2

@josevalim
Copy link
Contributor Author

(I think I'm misunderstanding something but any helps are appreciated. Also, I'm sorry about spreading around bad practices 😓.)

It was not a bad practice, it was the only way to do until more recent Elixir versions. :) I will investigate what is happening.

@josevalim
Copy link
Contributor Author

Ok, this has actually been a misunderstanding from my side. @preferred_cli_env only applies to tasks in the current project. I was not quite aware of this limitation before but I don't think we can fix it.

So I would recommend reverting my changes and adding the previous recommendations back to the README. I will update Elixir documentation to make this clearer. Sorry for the confusion! :(

@parroty
Copy link
Owner

parroty commented Mar 31, 2017

Thanks for checking! I'll update the README.

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