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

Use indexmap #25

Merged
merged 2 commits into from
Oct 2, 2023
Merged

Use indexmap #25

merged 2 commits into from
Oct 2, 2023

Conversation

Hoverbear
Copy link
Contributor

Description

The processed order and the output order should be as similar as possible, also the output ordering should be deterministic.

Checklist
  • Formatted with cargo fmt
  • Added or updated relevant tests (leave unchecked if not applicable)
  • Added or updated relevant documentation (leave unchecked if not applicable)
  • Linked to related issues (leave unchecked if not applicable)

@Hoverbear Hoverbear requested a review from cole-h October 2, 2023 16:36
@Hoverbear Hoverbear self-assigned this Oct 2, 2023

let map = res.unwrap();

// Ensure it's not just luck that it's the same order...
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if there's a better way to test for this... Right now, just transplanting this test into the current HashMap implementation, I get this to pass like 4/10 attempts...

Copy link
Contributor Author

@Hoverbear Hoverbear Oct 2, 2023

Choose a reason for hiding this comment

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

Short of asserting the type is an indexmap, I'm not sure how we'd do that. There isn't like a OrderedIterator marker or anything.

@Hoverbear Hoverbear merged commit 1dcd5d0 into main Oct 2, 2023
1 check passed
@cole-h cole-h deleted the use-indexmap branch October 2, 2023 21:21
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.

None yet

2 participants