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

Replace proc-macro-error with proc-macro-error2 #68

Merged
merged 1 commit into from
Oct 10, 2024

Conversation

Sympatron
Copy link
Contributor

proc-macro-error depends on syn 1.0 and is no longer maintained. This PR replaces it with the drop-in replacement proc-macro-error2.

Copy link
Member

@jannic jannic left a comment

Choose a reason for hiding this comment

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

I don't have any experience with proc-macro-error2, but it indeed looks like a maintained fork of proc-macro-error.

However, it's only 22 days old, so it's unclear for how long it will stay maintained.

Not saying that this change is bad - just not sure if it is a long-term solution. Any opinions?

@Sympatron
Copy link
Contributor Author

I thought about that too, but replacing it with more established crates like manyhow would require changes to the code base I could not do, because of my limited knowledge of proc macros.

The main reason I would want to update is to get rid of syn 1.0. That goal would be achieved independent of future maintenance. So If it becomes unmaintained in the future you could still switch to a different crate.

I primarily considered it to be ok because defmt did the same: knurling-rs/defmt@3a1c58a

@jannic
Copy link
Member

jannic commented Sep 27, 2024

I primarily considered it to be ok because defmt did the same: knurling-rs/defmt@3a1c58a

That's an interesting data point. And yes, completing the switch from syn 1 to syn 2 would be nice.

I'll still not merge this right now to give others a chance to comment, but that doesn't mean I don't like the change.

@jannic
Copy link
Member

jannic commented Oct 10, 2024

I'll still not merge this right now to give others a chance to comment, but that doesn't mean I don't like the change.

No further comments in about 2 weeks. With proc-macro-error2 quickly gaining popularity according to crates.io, I don't think there's a reason to wait any further. (And in case there are any objections, we can easily revert the commit.)

@jannic jannic merged commit c92f324 into rp-rs:main Oct 10, 2024
1 check passed
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