Add profile to rebar3 for erlang

[dgud]

To be able to compile test files profile needs to be set to "test",
with the "default" profile test files are not compiled.

Test files are found in the application "test" sub-directory.

[CLAassistant]

All committers have signed the CLA.

[dgud]

Or is there a better solution? @joedevivo @tsloughter

[dgud]

And I need some help with the travis errors, which I don't understand.

[tsloughter]

So this uses the test profile only if the file being modified is in a test directory?

[fmdkdd]

So this uses the test profile only if the file being modified is in a test directory?

Only if the file being modified is in a directory named "test" rather. My question is then: does Erlang/rebar forces to name your test directory "test", or are you free to name it something else?

[tsloughter]

rebar3 defaults to looking in "test" and Erlang convention is "test".

I don't know that I've seen a different directory used before. The only thing is this won't technically catch all modification of test code because some put unit tests directly in the module being tested with macros:

-ifdef(TEST).
... unit tests...
-endif.

TEST is set by the test profile. So if the user is modifying a unit test like this it would still compile in the default profile. But I don't think this should stop the change.

[fmdkdd]

TEST is set by the test profile. So if the user is modifying a unit test like this it would still compile in the default profile.

And there's no way to run the test profile alongside the default one? Or maybe the test profile is a superset of the default one?

[dgud]

I do think the only difference for rebar (when profile is test) is to define TEST macro and compile files (including files in test dir) but Tristan knows this better, I'm just guessing here.

[fmdkdd]

Why I'm asking is that the usual way we would provide the functionality of this PR would be through a new variable flycheck-rebar3-profile, which would be test or default. Then we would usually let a Flycheck extension (another package, which defines a function that would be added to flycheck-mode-hook) set the value of this variable depending on the current buffer (here, using the buffer filename).

The advantage of using a variable instead of the (eval (rebar3-profile)) is that a variable can be set manually, through buffer- or file-locals, and can be overridden easily.

The downside is that you would need to write and publish a separate extension, just for a very simple function, which seems a bit silly.

If you feel that there is no valid use case for customizing the behavior of the rebar3-profile, then we don't need a variable and can go through with your implementation.

[dgud]

Hmm, thinking about what you wrote, why can't we always run with the 'test' profile?
Could there be a problem with that @tsloughter ?

:command ("rebar3" "as" "test" "compile")

[tsloughter]

That is what I thought this PR was at first. I'm not sure it is a good default. Maybe most people will want it that way, either way if it is default or not it should be easily configurable.

[fmdkdd]

either way if it is default or not it should be easily configurable.

Then in that case we can more simply have a configurable variable for the profile to use, with default as the default value. No need to check the directory the file resides in.

[tsloughter]

Eh, I like the use of test profile if you are in the test directory. It should be configurable for those who want to always use the test profile, for instance if they include tests in modules directly.

[fmdkdd]

Okay so we want the profile to be user-configurable, and if the user doesn't say anything, we look at the directory like this PR does right now.

@dgud Can you add a flycheck-def-option-var for rebar3's profile? The default should be nil, and it acceptable values would be default and test (for the :type of defcustom). Then, in rebar3-profile, you would check that variable's value: if it's set, use that value for the profile, otherwise fallback on auto-detection using the directory name.

Sounds good?

[tsloughter]

Yup, that'd be great.

[dgud]

Yeah I can try, give me a couple of days, I've been sick so it will take a while until I caught up with things.

[fmdkdd]

@dgud No pressure; please take time to recover. This PR isn't going anywhere ;)

[dgud]

Some questions, are there more profiles we should support or are they user defined 'prod'/'native' @tsloughter ?

What should package-version be?

And please review my lisp code and comments I'm not fluent in either language :-)

[fmdkdd]

What should package-version be?

32. You can look at the top of the file: "32-cvs" means we are currently working towards version 32.

[fmdkdd]

You want to line feed after "test" and reindent in order to pass the make check test.

[fmdkdd]

Thanks for the changes. You'll need to document the new option in the manual as well to pass the CI. Add a defcustom line in doc/languages.rst (see other checkers around for examples).

[dgud]

Ah wait, do we want the user to be able to provide an arbitrary profile beyond default and test or not?

That was the question to @tsloughter ?

[fmdkdd]

That was the question to @tsloughter ?

Yes indeed. If we do want more profiles then we may want to use strings instead for the option, and allow for an arbitrary (string case. If we only care about test and default, then we can stick to symbols.

[tsloughter]

@fmdkdd that could be useful, yea, good idea.

[dgud]

Changed to arbitrary strings.

[fmdkdd]

LGTM. Please add a line to CHANGES.rst introducing the new option.

[dgud]

Added line to Changes, squashed commits and rebased to master.

[fmdkdd]

Nice work! Thank you. 👏