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 nightly imports #563

Merged
merged 4 commits into from
Mar 27, 2024
Merged

Conversation

tcoratger
Copy link
Collaborator

@tcoratger tcoratger marked this pull request as ready for review March 8, 2024 11:46
@xJonathanLEI
Copy link
Owner

Hi I don't quite understand this PR. When does it mean to "fix nightly imports"? The library should already work with nightly? It's checked on CI that nightly builds are successful. Or is it not working for no_std on nightly now?

@tcoratger
Copy link
Collaborator Author

Hi I don't quite understand this PR. When does it mean to "fix nightly imports"? The library should already work with nightly? It's checked on CI that nightly builds are successful. Or is it not working for no_std on nightly now?

@xJonathanLEI This is related to nightly target. In the CI and in the repo we are targeting Rust nigthly. Recently, new components have been added to the Rust std prelude, especially for nightly (https://doc.rust-lang.org/std/prelude/index.html) such as Vec for example. So there is a compilation warning on the reimports of these types now integrated into the prelude (you can see it on the CIs of the other PRs in progress). As we are targeting no_std on certain builds, we cannot simply remove these imports because they will no longer be recognized at all during no_std builds. Consequently, the simplest thing is to hide the import of the Vec in a vec::*, which is done in this PR for example. And as a result of that the full CI is ok.

@xJonathanLEI
Copy link
Owner

I see. So the problem is that we only run tests on nightly for CI, but not clippy. Clippy is currently only run for stable Rust. If we want to fix what's proposed in this PR we better also have it on CI to enforce that too tho.

@xJonathanLEI xJonathanLEI force-pushed the fix-nightly branch 2 times, most recently from d1be46d to c1427f6 Compare March 27, 2024 10:43
@xJonathanLEI
Copy link
Owner

Again, this PR changes codegen.rs, which is not supposed to happen. That file must not be manually edited.

@tcoratger
Copy link
Collaborator Author

Again, this PR changes codegen.rs, which is not supposed to happen. That file must not be manually edited.

Then I think two possibilities:

  1. We change the codebase that generates this file and update the PR.
  2. We merge as is and change the code generation just after the merge of the PR.

@xJonathanLEI xJonathanLEI merged commit 1ebcdc1 into xJonathanLEI:master Mar 27, 2024
26 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.

2 participants