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

Fix docsrs build error regarding IntoDiscriminant trait #414

Merged
merged 1 commit into from
Feb 15, 2025

Conversation

tveness
Copy link
Contributor

@tveness tveness commented Feb 12, 2025

The current build on docs.rs is failing.
This is due to the inclusion of IntoDiscriminant as a macro re-export.

This is a compilation error because IntoDiscriminant is not in the location expected by the DocumentMacroRexports macro. It's also semantically suspect as IntoDiscriminant is also not a macro, so it seems strange for it to be included in the list more generally.

The current PR resolves this by renaming IntoDiscriminant back to EnumDiscriminants, effectively reverting a change introduced in PR377. Perhaps @vpochapuis can comment on whether this is a good idea and if there are alternative approachs to resolving the issue.

Running cargo +nightly rustdoc --features derive -- --cfg docsrs in the strum directory is sufficient to reproduce the build failure, and confirms that current commit resolves the error. It could also be worth adding this to CI to ensure that docs builds don't fail in the future?

Aside: It would also be nice to somehow resolve the docs warnings of EnumDiscriminants not being available when compiling without the derive feature, and not sure if the IntoDiscriminant trait should be gated to prevent this, but I'm leaving this alone for the meantime as it's not really the issue at hand.

@vpochapuis
Copy link
Contributor

vpochapuis commented Feb 12, 2025

The current build on docs.rs is failing.
This is due to the inclusion of IntoDiscriminant as a macro re-export.

This is a compilation error because IntoDiscriminant is not in the location expected by the DocumentMacroRexports macro. It's also semantically suspect as IntoDiscriminant is also not a macro, so it seems strange for it to be included in the list more generally.

The current PR resolves this by renaming IntoDiscriminant back to EnumDiscriminants, effectively reverting a change introduced in PR377. Perhaps @vpochapuis can comment on whether this is a good idea and if there are alternative approachs to resolving the issue.

Running cargo +nightly rustdoc --features derive -- --cfg docsrs in the strum directory is sufficient to reproduce the build failure, and confirms that current commit resolves the error. It could also be worth adding this to CI to ensure that docs builds don't fail in the future?

Aside: It would also be nice to somehow resolve the docs warnings of EnumDiscriminants not being available when compiling without the derive feature, and not sure if the IntoDiscriminant trait should be gated to prevent this, but I'm leaving this alone for the meantime as it's not really the issue at hand.

Hello,
I don't have specific comments about this issue, when i applied #376 (comment) I didn't notice at the time the introduced bug. Its great to have found the issue and provide a fix! Thank you

@Peternator7 Peternator7 merged commit 9db3c4d into Peternator7:master Feb 15, 2025
1 check passed
@Peternator7
Copy link
Owner

Fixes #415

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.

3 participants