r3: simplify build #173
r3: simplify build #173
Conversation
Codecov Report
@@ Coverage Diff @@
## master #173 +/- ##
==========================================
+ Coverage 82.09% 82.12% +0.02%
==========================================
Files 13 13
Lines 3150 3149 -1
==========================================
Hits 2586 2586
+ Misses 564 563 -1
Continue to review full report at Codecov.
|
mix.exs
Outdated
| @@ -22,7 +22,7 @@ defmodule Proper.Mixfile do | |||
| end | |||
|
|
|||
| defp package do | |||
| [files: ~w(src include rebar.config configure Makefile COPYING README.md THANKS check_escripts.sh clean_doc.sh clean_temp.sh write_compile_flags mix.exs), | |||
| [files: ~w(src include rebar.config configure Makefile COPYING README.md THANKS check_escripts.sh clean_doc.sh clean_temp.sh flags mix.exs), | |||
There was a problem hiding this comment.
Not sure what the flags file is. Can you elaborate?
rebar.config
Outdated
| @@ -35,9 +37,22 @@ | |||
| {erl_opts, [debug_info, | |||
| report_warnings, {warn_format,1}, warn_export_vars, | |||
| warn_obsolete_guard, warn_unused_import, | |||
| warn_missing_spec, warn_untyped_record]}. | |||
| warn_missing_spec, warn_untyped_record, | |||
| warn_unused_vars, | |||
There was a problem hiding this comment.
I thought that warn_unused_vars was on by default... Do you have evidence to the contrary?
rebar.config
Outdated
|
|
||
| {dialyzer, [{warnings, [unmatched_returns | ||
| ]} | ||
| ,{plt_extra_apps, [erts, kernel, stdlib, compiler, crypto, syntax_tools]} |
There was a problem hiding this comment.
Please follow the style of the rest of the file.
rebar.config
Outdated
| ,{plt_extra_apps, [erts, kernel, stdlib, compiler, crypto, syntax_tools]} | ||
| ]}. | ||
|
|
||
| {profiles, [{test, [{erl_opts, [nowarn_missing_spec |
There was a problem hiding this comment.
Why are these nowarn_ options present here? They should not really be, since this PR is still using (our patched version of) rebar, isn't it?
There was a problem hiding this comment.
This is rebar3-specific and clears great many a warnings when running test.
You can see that removing the option and running rebar3 eunit is quite harsh.
There was a problem hiding this comment.
No, I do not want to see that. Instead, I want to see a pull request that does one thing, and one thing only, and does it right. This one is titled "Simplify build". Let's have that.
|
Please, do not include in this PR changes which are rebar3-specific, if you actually want it merged. I've made this point before but apparently it did not get across. I hope it does now. We want PRs that do one thing only and do that thing well. This is supposed to be a pull request that simplifies the build system. If you want to also substitute |
|
Last commit has been removed. This looks ready to me ;) |
Not to me though. I still do not understand why the entry {profiles, [{test, [{erl_opts, [nowarn_missing_spec
...
appears in According to your comment, this is rebar3-specific. So it should also be removed. |
|
Right. Just patched & squashed. |
This is the build system simplification extracted from #168
It makes use of the rebar
platform_defineto not require writing an HRL to the filesystem. This option is supported by rebar, rebar3 and the custom patched rebar that is bundled in this repo.The idea is to simplify/standardize the build process so that one day a custom patched rebar would not need to be part of this repo as well as making development easier long term for contributors that use rebar3.