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

Bump IndexMap to V2, resolve some Clippy lint suggestions #75

Conversation

martingallagher
Copy link

No description provided.

@ahl
Copy link
Collaborator

ahl commented Jun 28, 2023

it's disappointing that indexmap renamed the feature; otherwise we could just depend on a range of versions. As it is, I don't see how to update the indexmap dependency without it being a breaking change (i.e. moving this crate to 2.0). Any thoughts?

@martingallagher
Copy link
Author

martingallagher commented Jun 28, 2023

Maybe a re-export of indexmap, like the datafusion suite of crates? Datafusion re-exports sqlparser for example (its local dep is often out of sync with the latest sqlparser).

You're right about the pain involved, this PR is a consequence of trying to update indexmap on a project, I didn't fully consider the consequences. Perhaps this PR can be ignored and users can import a indexmap_v1 package alias, but going forward maybe the re-export pattern could be useful, I doubt I'm alone in wanting to update the indexmap version in my projects.

@ahl
Copy link
Collaborator

ahl commented Jun 28, 2023

100% with you -- I want to update indexmap elsewhere as well. Just trying to navigate past this in the least painful way. I wish that indexmap had retained the serde-1 feature for this case...

@ahl
Copy link
Collaborator

ahl commented Aug 28, 2023

I found a way to update the dependency in a non-breaking way: #76. Feel free to update this PR with the clippy changes if you'd like. Please let me know if my approach worked (or didn't work) for you.

@ahl ahl closed this Oct 20, 2023
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