Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add sortMaps:bool option #119

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

add sortMaps:bool option #119

wants to merge 4 commits into from

Conversation

cryptix
Copy link

@cryptix cryptix commented Dec 11, 2024

This starts the work on adding sortMap:bool to make it easy to create canonical map representations.

I had trouble resolving these two comments:

I don't really see the point in moving this to a different library. Since the comment on 4 was newer I just went ahead and did it to see how hard it would be.

My rationale for doing it this way would be:

  • re-creating a map/object with sorted keys is not very memory efficient either
  • it bubbles up the problem to parts of the stack that might not (want to) care about it
  • this code already iterates over keys list anyway, so we just need to sort those

This surely needs more tests before it can be merged/released but I wanted to get a temp check before I spent too much time on this.

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.

1 participant