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

Implement MutableOrderedMap.toImmutable(). Add ImmutableOrderedMapAdapter. #1696

Merged
merged 1 commit into from
Sep 28, 2024

Conversation

motlin
Copy link
Contributor

@motlin motlin commented Sep 1, 2024

No description provided.

donraab
donraab previously requested changes Sep 2, 2024
Copy link
Contributor

@donraab donraab left a comment

Choose a reason for hiding this comment

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

@motlin This PR needs to have supporting tests added before we can review and merge. Thanks!

@motlin
Copy link
Contributor Author

motlin commented Sep 2, 2024

@motlin This PR needs to have supporting tests added before we can review and merge. Thanks!

Good call, I added some tests. I'm finding it tricky to land the changes to improve maps in any order. The massive code review is #1617 and it keeps shrinking as I land other PRs. In the meantime, this PR adds a new feature, but also increases code duplication. All the tests are identical to ones for unmodifiable tests, but the maps are not yet consistent enough to have a single test hierarchy.

@motlin motlin requested a review from donraab September 10, 2024 17:40
@motlin motlin force-pushed the ImmutableOrderedMapAdapter branch 2 times, most recently from eb5069d to 6ee3433 Compare September 15, 2024 20:05
@motlin motlin dismissed donraab’s stale review September 23, 2024 22:01

Stale request for changes, all suggestions incorporated.

Copy link
Contributor

@donraab donraab left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for adding this @motlin!

@motlin motlin merged commit bc0cf82 into eclipse:master Sep 28, 2024
14 checks passed
@motlin motlin deleted the ImmutableOrderedMapAdapter branch September 28, 2024 14:00
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