Don't instrument functions by default

[martinklepsch]

cli-matic.core calls orchestra.spec.test/instrument when being loaded. This instruments all spec'ed functions and not just cli-matic's own API. In my case this brought up surprising issues where I didn't expect them.

Would you consider a PR where only cli-matic's own functions are instrumented?

[l3nz]

Hi Martin, I thought it would only instrument the current namespace? but maybe I was wrong. MAybe there is away to only instrument the current ns? Or other things that are better than just putting an ; before?

(BTW, building docs breaks, so I had to go back to Github - https://circleci.com/gh/martinklepsch/cljdoc-builder/6870 )

[martinklepsch]

I think you can supply a list of vars to instrument to the instrument function.

That Orchestra build is failing is a known issue that requires some changes to the CLJS analysis code (https://github.com/cljdoc/cljdoc/issues/201).

[l3nz]

I think it would make sense.

[l3nz]

You want to submit a PR or you want to wait for me to de-instrument?

[martinklepsch]

Here's a list of symbols that could be passed to instrument:

['mk-cli-option 'get-subcommand 'all-subcommands-aliases
'all-subcommands 'canonicalize-subcommand 'rewrite-opts
'list-positional-parms 'a-positional-parm 'capture-positional-parms
'generate-global-command-list 'generate-global-help
'generate-subcmd-help 'mkError 'parse-single-arg
'errors-for-missing-mandatory-args 'mk-fake-args
'parse-cmds-with-defaults 'check-one-spec 'check-specs-on-parameters
'check-specs-on-parsed-args 'parse-cmds 'assert-unique-values
'assert-cfg-sanity '->RV]

Won't prepare a PR for now but maybe this helps. Also it might be better to just tackle #22 instead.

[l3nz]

My problem is maybe wrongly stated: I would like not to have a special way to know if I'm developing (and I want Orchestra there) or not. By including (orchestra/instrument) in the namespace, I mean to re-instrument everything on reload - I do a change, CMD+SHIFT+M, and it's already there... I do not necessarily want it in production. But how do we handle this case? that's where #22 came from.

Maybe as a stopgap we could do (instrument (list-all-fns-in-current-ns)), but my real issue is the one above. Listing all fns in the namespace should be trivial anyway - see https://stackoverflow.com/questions/2747294/how-to-list-the-functions-of-a-namespace

[l3nz]

Stoopid me - we could have a fn that checks if Orchestra is present, and instrument is it's there, and just do nothing if it's not on the classpath. This would probably be even better.

[l3nz]

I think that version 0.2.0 is out now. Try it!

[martinklepsch]

The changes will still instrument all instrumentable functions which I think could be surprising to users of this library. It might still be worth considering something like (instrument (list-all-fns-in-current-ns)).