-
Notifications
You must be signed in to change notification settings - Fork 17
close #6 - change map to unordered #7
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
Conversation
|
@redis/core-team please approve this change to the spec. |
oranagra
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i approve.
we kinda already dealt with this here: redis/redis#8391
realizing that we may want to stay away from that and never rely on map being ordered.
|
Should we add it to the "version history" section at the top? |
|
yes, it also occurred to me. it's probably wrong to just modify a word in the spec without noting anywhere that it was changed. maybe use this opportunity to mention something about the aggregated types. |
zuiderkwast
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merge this?
Regarding version history, if we merge more PRs at the same time, we can have one version history bullet combining the changes.
|
i added two links in the top about the places were we realized where map should be ordered (included a breaking change in 6.2). |
itamarhaber
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| was made. | ||
| * 1.3, 11 Mar 2019, Streamed strings and streamed aggregated types. | ||
| * 1.4, 8 Dec 2022, Normalize NaN to a single representation "nan" and forbid "-nan" (Effective since Redis 7.2). | ||
| * 1.4, 5 Jan 2023, Mark the map type as being unordered rather than ordered (Effective since Redis 6.2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oranagra Sorry I was out today.
I see that now we have two 1.4 releases with different dates...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks. i'll fix.
see:
close #6