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

Re-export native_model, closes #301 #302

Merged

Conversation

imbolc
Copy link
Contributor

@imbolc imbolc commented Jan 4, 2025

Imports look weird now. native_model::self is necessary because #[native_model] macro assumes its visibility.

I've tried pub use native_model::{native_model, Model}, which would simplify the imports, but it's not easy to re-export a proc macro and there's a conflicting pub use model::Model.

So, I'm ok if you change your mind on the re-exporting, but I still think it's better than manually fitting versions.

@imbolc imbolc force-pushed the reexport-native_model branch from 0b822b2 to ab946b6 Compare January 4, 2025 05:03
@imbolc
Copy link
Contributor Author

imbolc commented Jan 4, 2025

@vincent-herlemont What should I do about the failing Release Automatic / wait-for-workflows check?

@imbolc imbolc force-pushed the reexport-native_model branch from ab946b6 to 67f7e4a Compare January 4, 2025 05:33
@vincent-herlemont
Copy link
Owner

@imbolc Thank you for your PR!

Just a note, could you remove this line version_update.sh#L44 that updates the native_model version in the readme?

@vincent-herlemont
Copy link
Owner

Imports look weird now. native_model::self is necessary because #[native_model] macro assumes its visibility.

I've tried pub use native_model::{native_model, Model}, which would simplify the imports, but it's not easy to re-export a proc macro and there's a conflicting pub use model::Model.

@imbolc We'll assess the situation before proceeding with the release. I'll evaluate how significant the issue is. For the time being, let's implement your solution.

@vincent-herlemont vincent-herlemont linked an issue Jan 5, 2025 that may be closed by this pull request
@vincent-herlemont
Copy link
Owner

vincent-herlemont commented Jan 5, 2025

@vincent-herlemont What should I do about the failing Release Automatic / wait-for-workflows check?

@imbolc This refers to this #161 I need to take care of it. However, for now, you can ignore this issue.

@imbolc
Copy link
Contributor Author

imbolc commented Jan 5, 2025

@imbolc Thank you for your PR!

Just a note, could you remove this line version_update.sh#L44 that updates the native_model version in the readme?

Done

@vincent-herlemont vincent-herlemont merged commit 48972fa into vincent-herlemont:main Jan 5, 2025
8 of 9 checks 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.

Reexporting native_model
2 participants