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

Leiningen approach can be refined #79

Closed
vemv opened this issue May 30, 2021 · 1 comment · Fixed by #80
Closed

Leiningen approach can be refined #79

vemv opened this issue May 30, 2021 · 1 comment · Fixed by #80

Comments

@vemv
Copy link
Contributor

vemv commented May 30, 2021

Checking out https://github.com/liquidz/antq/blob/400215d5e3edf61ac2cb3b993b9477ac7e69a061/src/antq/dep/leiningen.clj, the approach to checking project.clj doesn't quite seem idiomatic. project.clj is not data, but code meant to evaluated in a specific context. Treating it as data results in reduced accuracy and flexibility.

Three specific items can be pointed out:

  • eval won't work (one can use ~ to eval arbitrary Clojure code anywhere within project.clj)
  • profiles cannot be freely composed
  • plugins/middleware, that can always alter or inject extra dependencies, won't be taken into account.

Therefore I'd suggest that the current approach becomes deprecated. Instead a Lein plugin would be offered.

As usual with plugins, they receive a project argument with all evaluation rules already applied. So you don't need to worry about eval, profiles, or plugins - all those are already handled by Lein itself prior to one's plugin invocation.

WDYT?

I could contribute a PR.

@liquidz
Copy link
Owner

liquidz commented May 31, 2021

@vemv Thanks for your suggestion!

Therefore I'd suggest that the current approach becomes deprecated. Instead a Lein plugin would be offered.

I'm not going to deprecate the current approach because I want to keep a way to run it without Leiningen, but I know it's not accurate as you said.
So I agree with adding the Leiningen plugin as a more accurate way while keep the existing approach.

I could contribute a PR.

Great! PR is very welcomed.

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 a pull request may close this issue.

2 participants