Skip to content

Conversation

@leibale
Copy link
Contributor

@leibale leibale commented Aug 8, 2022

@itamarhaber
Copy link
Member

@redis/core-team please approve this change to the spec.

Copy link
Member

@oranagra oranagra left a 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.

@leibale
Copy link
Contributor Author

leibale commented Aug 9, 2022

Should we add it to the "version history" section at the top?

@oranagra
Copy link
Member

oranagra commented Aug 9, 2022

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.
let's add some history section, and mention that the original spec said it was ordered but it was decided to change it since some clients / languages won't be able to deal with that.

maybe use this opportunity to mention something about the aggregated types.

Copy link
Contributor

@zuiderkwast zuiderkwast left a 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.

@oranagra
Copy link
Member

oranagra commented Jan 5, 2023

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).
@zuiderkwast please take a quick look and i'll merge it.

Copy link
Member

@itamarhaber itamarhaber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@oranagra oranagra merged commit dbee7ab into redis:master Jan 5, 2023
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)
Copy link
Contributor

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...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks. i'll fix.

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.

Is map type unordered?

4 participants