Skip to content

Conversation

@bsless
Copy link
Contributor

@bsless bsless commented Mar 19, 2024

Speeds up :long benchmark ~2.5x and :mixed ~2x

Closes #87

@brandonbloom
Copy link
Owner

Thanks for the PR and making the changes! Hopefully I'll have a chance to follow-up / validate next weekend.

@bsless
Copy link
Contributor Author

bsless commented Mar 25, 2024

Alright, applied all requested changes and fixes (including missed typo)

@bsless bsless requested a review from brandonbloom March 27, 2024 15:49
@bsless
Copy link
Contributor Author

bsless commented Apr 8, 2024

Gentle bump :)

@brandonbloom
Copy link
Owner

I haven't forgotten about you. I only really get to work on OSS on weekends, and not every weekend. Thanks for your patience.

@bsless
Copy link
Contributor Author

bsless commented Dec 9, 2024

Hi @brandonbloom, following up, it's been a while - are there any other changes you'd like me to make in the PR?

@bsless
Copy link
Contributor Author

bsless commented Mar 20, 2025

Hey, following up again on the PR's first birthday, typo has been fixed long time ago and awaiting your review.
Any other changes you'd like me to make?

@brandonbloom
Copy link
Owner

These changes seem to break the ClojureScript tests. Please run ./test.sh locally.

Example failure:

ERROR in (pprint-edn) (RangeError:NaN:NaN)
Pretty printing Edn without metadata
expected: (= (clean (with-out-str (pprint data {:width 70}))) wide)
  actual: #object[RangeError RangeError: Maximum call stack size exceeded]

@bsless
Copy link
Contributor Author

bsless commented Jun 29, 2025

@brandonbloom got around to fixing it, turns out I just needed to increase node's stack size.
Also found out I should have used keyword-identical? thanks to Thomas Heller.

@brandonbloom brandonbloom merged commit 2ea338e into brandonbloom:master Jul 16, 2025
0 of 6 checks passed
@brandonbloom
Copy link
Owner

This PR has been included in release v0.6.28! 🎉

brandonbloom added a commit that referenced this pull request Jul 26, 2025
brandonbloom added a commit that referenced this pull request Jul 26, 2025
Compare to #89 and note
breakage #94
@brandonbloom
Copy link
Owner

This had to be reverted and fixed a different way. See #94

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fipp.ednize/override? uses satisfies?

2 participants