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

rename test task to avoid collision #48

Open
instilled opened this issue Dec 24, 2015 · 3 comments
Open

rename test task to avoid collision #48

instilled opened this issue Dec 24, 2015 · 3 comments

Comments

@instilled
Copy link

Martin,

First of all great work with tenzing. Awesome bootstrap for cljs with boot!

Just skimmed the generated build.boot and noticed your comment about the name collision:

;;; This prevents a name collision WARNING between the test task and
;;; clojure.core/test, a function that nobody really uses or cares
;;; about.
(ns-unmap 'boot.user 'test)
(deftask test []
  (comp (testing)
        (test-cljs :js-env :phantom
                   :exit?  true)))

Did you know about replace-task! in boot? With the following you can easily redefine an existing task:

(replace-task!
 [t test] (fn [& xs] (comp (testing) (test-cljs :js-env :phantom :exit?  true))))

;; or if you still want to invoke the original, redefined task something along the lines:
;;(replace-task! [t test] (fn [& xs] (comp (testing) (apply t xs))))

Anyways,
Thansk for the great work!

@crisptrutski
Copy link
Collaborator

Hey Martin Fabio (woah, too many festive beverages)

Thanks for the suggestion, I hadn't seen this replace-task! macro before - it's pretty groovy.

One thing to note is that clojure.core/test is not actually a task at all. It's a fairly obscure function that could make some sense from the REPL, but has nothing to do with regular unit test runners, or boot tasks.

It seems to me this replace-task! macro is just sugar for decorating tasks, and a bit clumsy for defining something novel. You're not actually generating the CLI metadata, or using the required binding variable.

The warning and unmap mechanism you quote above is fairly "orthodox", being taken from the official boot-test repo.

On the other hand, I agree that it's a fairly technical, scare-quotey piece of code to place in such a prominent position for users, guessing that was your motivation opening this. While I don't think replace-task! is the improvement we'd want to apply, I'm interested in a better solution.

Perhaps 'boot.user should even come with this var stripped by default?

@crisptrutski
Copy link
Collaborator

Whoops, didn't mean to close! A clumsy trackpad fumble 😝

@martinklepsch
Copy link
Owner

We could also rename the task. I often use test! as a task name to avoid collision.

@martinklepsch martinklepsch changed the title boot redefine-task! to avoid warnings rename test task to avoid collision Dec 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants