Fork me on GitHub
#cljs-dev
<
2015-07-26
>
juhoteperi17:07:12

I have some problems with getting analyze to produce warnings, this test is also failing: https://github.com/clojure/clojurescript/blob/master/src/test/clojure/cljs/analyzer_tests.clj#L31

juhoteperi17:07:32

Oh, it's the undeclared-var warning which doesn't work

bensu17:07:20

@juhoteperi: I just pulled a fresh master and as you said, the test is failing! There is a standing issue for the other 2 failing tests: http://dev.clojure.org/jira/browse/CLJS-1349 but none for the no-warn you just discovered. Would you mind filing one?

juhoteperi17:07:38

@bensu: all-warn, yeah I'll do that

bensu17:07:35

@juhoteperi: sorry, all-warn. great!

juhoteperi18:07:40

Created. I found the problem commit with bisect also.

bensu19:07:08

@juhoteperi: thanks for going the extra mile with the bisect. Is this something you want to pursue or should I assign it to me?

juhoteperi19:07:12

@bensu: Feel free to take it. I looked into it quickly and couldn't see the cause straight away.

bensu19:07:21

@juhoteperi: ok. I'll get it during the week so we can discuss the more interesting cljs-1367

bensu19:07:44

@juhoteperi: I've been thinking about flags and analyzer.api, what would be the best to allow analyzer.api users to have some control over the flags, without exposing them directly since their impl might change.

bensu19:07:34

one option is to pass an opts map as an interface to those flags, including the warning-handlers.

juhoteperi19:07:43

I'm mostly interested in using warning handlers together with cljs.build.api/build

juhoteperi19:07:57

So if the warning handlers were passed in as argument, they would need to be added elsewhere too

juhoteperi19:07:55

The most useful api for me would be if build (and others) returned useful value containing e.g. number of warnings

bensu19:07:35

@juhoteperi: can you point me to the code that needs the number of warnings?

juhoteperi19:07:33

If warning count is non-zero Boot can e.g. play sound notification

bensu19:07:23

@juhoteperi: thanks, it makes sense.