format: make groupIterable sort by row #3851
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
srenatus
commented
Oct 4, 2021
| globals = { | ||
| "foo": "bar", | ||
| "fizz": "buzz", | ||
| } |
There was a problem hiding this comment.
This reverts the crucial bit of #3823 that should have given this bug away but that I didn't notice in its full extent.
srenatus
force-pushed
the
sr/issue-3849/fmt-change
branch
from
f93c957
to
ddac31f
Compare
This was referenced Oct 4, 2021
Before, it depended on the elements being passed in ordered by their rows. Before open-policy-agent#3823, the iteration order was the same as the row order; but with sorting the keys slice (which determines iteration order) on creation, that was changed. Now, we'll sort the elements within `groupIterable`. Fixes open-policy-agent#3849. Signed-off-by: Stephan Renatus <[email protected]>
Since map iteration is randomized in golang, this benchmark didn't actually measure what was intended, but rather the presence or ab- sence of duplicate keys. Now, we'll create a set of keys to insert before, and either shuffle it or use it in its increasing order: to call `(Object).Insert()` in a loop. Signed-off-by: Stephan Renatus <[email protected]>
srenatus
force-pushed
the
sr/issue-3849/fmt-change
branch
from
cd75610
to
38c5e53
Compare
|
Added the forward-ported CHANGELOG and capabilities from 0.33.1. |
Signed-off-by: Stephan Renatus <[email protected]>
srenatus
force-pushed
the
sr/issue-3849/fmt-change
branch
from
38c5e53
to
5b324c8
Compare
|
Approved in #3854 ✅ |
dolevf
pushed a commit
to dolevf/opa
that referenced
this pull request
Nov 4, 2021
* format: make groupIterable sort by row Before, it depended on the elements being passed in ordered by their rows. Before open-policy-agent#3823, the iteration order was the same as the row order; but with sorting the keys slice (which determines iteration order) on creation, that was changed. Now, we'll sort the elements within `groupIterable`. Fixes open-policy-agent#3849. Also includes: * ast/term_bench_test: fix benchmark Since map iteration is randomized in golang, this benchmark didn't actually measure what was intended, but rather the presence or ab- sence of duplicate keys. Now, we'll create a set of keys to insert before, and either shuffle it or use it in its increasing order: to call `(Object).Insert()` in a loop. * CHANGELOG/capabilities: update for v0.33.1 bugfix release Signed-off-by: Stephan Renatus <[email protected]> Signed-off-by: Dolev Farhi <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before, it depended on the elements being passed in ordered by their rows.
Before #3823, the iteration
order was the same as the row order; but with sorting the keys slice (which
determines iteration order) on creation, that was changed.
Now, we'll sort the elements within
groupIterable.Fixes #3849.
For #3849, this means that the ordering that matters for formatting literal objects in rego code is
the row ordering. This PR reverts the behavioural change that would make the formatter print those
with their keys sorted.