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

Gen sorted interfaces #862

Merged
merged 2 commits into from
Dec 12, 2022
Merged

Conversation

endgame
Copy link
Collaborator

@endgame endgame commented Dec 9, 2022

Switch the generator from HashMap to Map, which means that all the toList calls come out in sorted order. This will keep the ordering of fields predictable and should give us better diffs in future generator runs.

While it might be nice to follow botocore's ordering for struct fields, I can't see a good way to do that: haskell/aeson#984

A full regeneration is on the endgame:gen-sorted-interfaces-gen branch (#863). Please try it and let me know if you think this is better than the arbitrary "however HashMap.toList sorted them" that we used to have.

Closes #825 .

This allows us to retrieve field names etc in sorted order.

Timings:

with Data.Map.Strict
real	2m0.380s
user	1m56.609s
sys	0m3.100s

with Data.HashMap.Strict
real	2m0.435s
user	1m56.775s
sys	0m3.017s
This makes explicit the effect of using Map instead of HashMap: that
all fields are emitted in sorted order.
@endgame endgame mentioned this pull request Dec 9, 2022
@endgame
Copy link
Collaborator Author

endgame commented Dec 10, 2022

It seems unlikely that aeson will gain the ability to do sorted Objects any time soon, so I think that makes sorted fields a better way to go.

@mbj
Copy link

mbj commented Dec 12, 2022

Please try it and let me know if you think this is better than the arbitrary "however HashMap.toList sorted them" that we used to have.

I did not try the branch. But determinism that does not depend on an implementation artifact (that in case of HashMap should actually not be that stable, to avoid collision / DOS attacs!) is better.

Given we cannot (easily) retain the source field order ATM, I think sorting by field name is the best compromise.

Copy link

@michaelwebb76 michaelwebb76 left a comment

Choose a reason for hiding this comment

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

Seems sensible enough to me, as a relative Haskell n00b

@endgame endgame merged commit 33dc8f8 into brendanhay:main Dec 12, 2022
@endgame endgame deleted the gen-sorted-interfaces branch December 12, 2022 06:39
endgame added a commit to endgame/amazonka that referenced this pull request Jan 17, 2023
Same output as brendanhay#862, but
sticks to `HashMap` internally to avoid triggering
https://github.com/brendanhay/amazonka/issue/888

A couple of enums change value in a strange way, because the generator
cannot handle enums-of-numeric values. See
brendanhay#889
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.

gen: Stop shape field names from jumping about
3 participants