Fork me on GitHub
#malli
<
2022-10-18
>
ikitommi05:10:35

[metosin/malli "0.9.2"], thanks @danvingo for the quick fix!

ikitommi05:10:20

would be good to have tests for the dev-tooling too for the future.

dvingo12:10:40

agreed! I will take a look into adding some for start!

dvingo22:10:58

@ikitommi I'm writing some tests for ! and friends and noticed that the arguments are a little confusing. start! takes :ns which indicates the set of namespaces to be collected, but then instrument! doesn't use that set - instead you have to separately pass :filters with [(mi/-filter-ns...) do you think it makes sense to use the set of namespaces passed in to start! if they are present and pass those to filters so instrument! will use the same set? Otherwise you have to pass both :ns and :filters to only start collection and instrumentation on the desired set of namespaces

ikitommi05:10:26

:thinking_face: just looked at the code, dev/start! doesn't seem to use any :ns option either. Where did you pick that up?

dvingo11:10:06

oh wow, that's my mistake! I see what happened - it's collect! that uses it, https://github.com/metosin/malli/blob/546eb663484e66ef1271b99189b17dbeca215ec8/src/malli/dev.clj#L23 which does support changing what is collected: https://github.com/metosin/malli/blob/546eb663484e66ef1271b99189b17dbeca215ec8/src/malli/instrument.clj#L46 I must have saw that and inferred that you wouldn't always want to collect everything when using dev/start! do you have any thoughts on if the :ns option should be supported for start!? I can change it back for the cljs version if you want

ikitommi06:10:19

I think it should in start! too, but silly to be only on collect!