clj-commons / byte-streams

A Rosetta stone for JVM byte representations
417 stars 33 forks source link

Run def-conversions only once #27

Closed jeaye closed 7 years ago

jeaye commented 7 years ago

Fix #26

Adding ?w=1 will rule out the whitespace differences and show just the couple of lines that changed. Here's a link: https://github.com/ztellman/byte-streams/pull/27/files?w=1

jeaye commented 7 years ago

Adding a regression test. Gimme a bit before merging, please.

jeaye commented 7 years ago

Ok, all set on my end. A new version deployed, after merging this, would be much appreciated. Then I can start to propagate the version changes up to jerks-whistling-tunes and then ring-gatekeeper. :)

jeaye commented 7 years ago

@ztellman Any chance of getting a release soon?

ztellman commented 7 years ago

Just cut 0.2.3-alpha1, sorry for the delay. Please feel free to follow up sooner in the future, this just completely slipped my mind.

jeaye commented 7 years ago

@ztellman Thank you. For how long are byte-streams releases typically in alpha? I'm wondering if I should hold off on the PRs to other libs until 0.2.3 is out.

ztellman commented 7 years ago

Which other libraries? The :reload-all workflow is not a typical one (for basically the reasons we ran into here), and it can be fixed by overwriting the dependencies in your project if you need it fixed. If this is affecting lots of people, I'm happy to cut a new release, but usually I'd wait for a few other changes to accumulate.

jeaye commented 7 years ago

I have been waiting to update https://github.com/aprobus/jerks-whistling-tunes/ (some discussion here) to resolve this issue so that I can ultimately update https://github.com/jeaye/ring-gatekeeper (and, ideally, its upstream).

The :reload-all workflow is essential for any non-trivial, repl-driven server development; I don't see how else it would work.

ztellman commented 7 years ago

I just update single namespaces, and avoid anything which holds onto references to older implementations. Perhaps I'm in the minority there, however. I have no objection to cutting a full release, can you just double-check that this alpha release resolves all your issues?

jeaye commented 7 years ago

Part of the reason that we take this :reload-all approach is due to clojure.spec. Since we have specs for the vast majority of our functions and data, and changing some code may invalidate previous specs, or add new specs, having a single restart! function in a my-proj.dev namespace, which does a :reload-all and then re-instruments the code with the new specs (using orchestra) means that we're always running with up-to-date instrumentation for our specs. I appreciate you taking the time to understand this.

With all of that said, I have verified that 0.2.2 has the issue and 0.2.3-alpha1 does not. :)

jeaye commented 7 years ago

@ztellman What's the verdict on this?

ztellman commented 7 years ago

Sorry, I keep getting side-tracked. I'll cut the release now.

ztellman commented 7 years ago

0.2.3 is available, sorry for the delay.

jeaye commented 7 years ago

Thank you!