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

Made some structs sortable and deserializable #415

Merged
merged 1 commit into from
Aug 25, 2021
Merged

Made some structs sortable and deserializable #415

merged 1 commit into from
Aug 25, 2021

Conversation

UebelAndre
Copy link
Collaborator

This is incredibly useful when using cargo-raze as a library.

@UebelAndre
Copy link
Collaborator Author

@dfreese would you be able to take a look at this?

@UebelAndre
Copy link
Collaborator Author

Actually, this can wait until #282 goes in. I'd rather not risk a conflict there.

@dfreese
Copy link
Collaborator

dfreese commented May 9, 2021

I have reservations with how we'd need to support this sort of use case. Since the cargo-raze library is only intended to be used by the cargo-raze-bin target, these are only public for the purposes of main.

Specifically with this change, the change from HashMap to BTreeMap is a pessimization, since we don't need the ordering. That would also be signal to me, later on, that order is important.

The use-case for Deserialize is fairly obvious to me, but can you explain why you'd need CrateSettings to be ordered?

@UebelAndre
Copy link
Collaborator Author

The use-case for Deserialize is fairly obvious to me, but can you explain why you'd need CrateSettings to be ordered?

It's so crate_universe can render lockfiles with minimal diffs when things update. This is accomplished by sorting the output which includes CrateSettings. bazelbuild/rules_rust#692

@dfreese
Copy link
Collaborator

dfreese commented Aug 9, 2021

Sorry for the long delay. #425 (an update of #282) has been merged, so I'm good with this being merged if you'd want to update it to latest main.

@UebelAndre
Copy link
Collaborator Author

Rebased and merging per #415 (comment)

@UebelAndre UebelAndre merged commit df6d94b into google:main Aug 25, 2021
@UebelAndre UebelAndre deleted the serde branch August 25, 2021 17:56
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.

2 participants