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

read rebar.config elvis section if exists #307

Closed
Licenser opened this issue Dec 7, 2015 · 11 comments
Closed

read rebar.config elvis section if exists #307

Licenser opened this issue Dec 7, 2015 · 11 comments

Comments

@Licenser
Copy link
Contributor

Licenser commented Dec 7, 2015

It would be great if elvis would look at the rebar.confg elvis section if no elvis.config existed. :)

@bullno1
Copy link
Contributor

bullno1 commented Dec 8, 2015

I think they are different tools which can exist independently so elvis should not look at rebar.config for its own config. A programmatic interface would make it easy to write a plugin for rebar.

@Licenser
Copy link
Contributor Author

Licenser commented Dec 8, 2015

Oh on the host side I fully agree, already exists ;) did that.

What I'm thinking about is the hosted elvis that is offered where the programmer has no control over how it is fetched.

@elbrujohalcon
Copy link
Member

@Licenser you mean the service at elvis.inakalabs.com ? It already reads the elvis.config file from your repo, if you push it. Why do you think it should check the rebar.config as well?

@Licenser
Copy link
Contributor Author

Licenser commented Dec 9, 2015

Mostly convenience, the canonical way of rebar is to combine the configurations of all plugins/tasks into the rebar.config file. so the elvis plugin for rebar uses the rebar.config file for the configuration. That can be argued good or bad, I can see valid arguments for both sides, but it seems to be the standard way.

@waisbrot
Copy link
Contributor

I think the consistent behavior would be that rebar3 elvis should use elvis config from rebar.config (if present). That's how relx works -- relx itself doesn't go looking in rebar.config but rebar3 release will fetch whatever you've put in the relx entry and pass that along.

Making an "elvis" plug-in for rebar sounds like a nice idea.

@Licenser
Copy link
Contributor Author

@waisbrot that plugin exists https://github.com/project-fifo/rebar3_lint and that works as you described (since it feels like the 'right' way for a plugin). The issue here (quite literally ;) is that this makes the hosted elvis service incompatible. Or rather it makes the plugin incompatible with the service.

While the incompatibility clearly comes from my plugin, my take is that having a consistent workflow for developers is important so it feels 'better' to adjust the service to read the config then to make the plugin read an extra config. That said I totally understand if the change like that is not in inaka's interest, it's a free service and I'm grateful for it one way or another.

@Licenser
Copy link
Contributor Author

Basically there are a few options:

  1. don't use the hosted elvis with the rebar3 lint plugin (least work)
  2. use the elvis.config file with with the plugin (kind of fair since it's 'my fault' that the plugin uses the rebar.config, but not intuitive for people using rebar)
  3. update the hosted elvis to read the config from rebar.config when a elvis.config doesn't exist (nicest from a integration perspective)
  4. set up a second service that works for the rebar.config (rather silly)
  5. keep both a elvis.config and a elvis action in the rebar.config (quite problematic)

PS: I'm happy to PR this if it's wanted.

@elbrujohalcon
Copy link
Member

@Licenser even if you want to send a PR you'll probably not be able to get it to the hosted server, mostly because the hosted service code lives in a private repo.
I'll be happy to accept a PR in this repo, but then I'll still have to open an issue in the private repo to include the fix there. And find some time off from some @inaka employee to work on that (which will be the hardest part).
I tell you what I will do: I'll start by adding an issue on the private repo with a link to this one and talk to my bosses to see when I can kidnap get some dev to work on that.

@Licenser
Copy link
Contributor Author

Sounds wonderful, I totally get that it's not a priority as it has no extra value to inaka in itself, so I'm happy to do as much of the work as possible :)

@elbrujohalcon
Copy link
Member

@Licenser we'll go with (3) in your list, @Euen will implement it on server side.

@elbrujohalcon
Copy link
Member

@Licenser This was fixed on inaka/elvis_core#33, can you give it a try and let us know if you need something else :)

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

No branches or pull requests

4 participants