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

Extend LEIN_SILENT to cljsbuild (#417) #418

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Extend LEIN_SILENT to cljsbuild (#417) #418

wants to merge 1 commit into from

Conversation

tmarble
Copy link

@tmarble tmarble commented Oct 11, 2015

Per the comment in #417 I have extended the LEIN_SILENT support to lein-cljsbuild.

Please note this introduces an additional, explicit dependency of cljsbuild on [leiningen-core "2.5.3"] which may be problematic (the proposed delta for lein-cljsbuild doesn't change the dependencies due to :eval-in-leiningen true).

NOTE: there remains this one non-Warning and non-Error println not converted (but perhaps should be?)

NOTE: There are whitespace changes in support/project.clj (due to clojure-mode and my Emacs (add-hook 'before-save-hook 'delete-trailing-whitespace)). The other significant change is change in scope of org.clojure/clojure to "provided".

@danielcompton
Copy link
Contributor

This PR also adds a dependency on Leiningen in the support plugin. While it may or may not be an issue for the Leiningen plugin itself, I suspect this dependency will be unwanted by any other project that wants to use cljsbuild in a programmatic fashion. There are also other usages of println used throughout the support project which haven't been converted yet?

@tmarble
Copy link
Author

tmarble commented Oct 11, 2015

Right, hence my comments above.

One could argue that we add a hack like this at the top of compiler.clj, but I'm not sure it's more elegant:

(let [lmain-info (ns-resolve (the-ns 'leiningen.core.main) 'info)]
  (def info (or lmain-info println)))

@mneise
Copy link
Collaborator

mneise commented Oct 11, 2015

I agree with @danielcompton that there is quite a bit of overlap with #419 which would make this PR redundant. The approach of having default loggers in #419 seems like a good idea which would save us from adding an additional dependency to the support project.

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.

3 participants