metosin / malli

High-performance data-driven data specification library for Clojure/Script.
Eclipse Public License 2.0
1.44k stars 204 forks source link

Delete old cljs instrumentation code #1013

Closed ikitommi closed 3 months ago

ikitommi commented 4 months ago

test break, see https://github.com/metosin/malli/pull/890

dvingo commented 4 months ago

There is one use-case in favor of keeping this around (but maybe renaming the namespaces?), which is for the use-case where you would want to run instrumented code with advanced compilation (and during development would still use the new namespacemalli.instrument)

Just came up on slack: https://clojurians.slack.com/archives/CLDK6MFMK/p1709638318412439

ikitommi commented 4 months ago

thanks, was about to ping you if this can be removed. But this doesn't work with latest versions of everything, as mentioned in Slack. Options:

1) remove it 2) fix it and maybe rename?

WDYT? would you have time to do the 2?

ingesolvoll commented 4 months ago

@ikitommi @dvingo I'm in the process of updating this code to use it for instrumenting my production build during E2E testing. But I'm perfectly fine with inlining the legacy malli code in my codebase, doesn't seem like there's much reason to keep it around here.

ingesolvoll commented 4 months ago

That said, looks like the only thing not working is the goog.mixin thing, which has a trivial fix.

ingesolvoll commented 3 months ago

1016 fixes the goog/mixin issue, and code works for me with that fix

ikitommi commented 3 months ago

closing this.