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.

Sign up for GitHub

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

Jump to bottom

Support no_std #39

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Support no_std #39

wants to merge 1 commit into from

Conversation

waywardmonkeys
Copy link
Collaborator

No description provided.

@waywardmonkeys
Copy link
Collaborator Author

For this to work, we'd need a semver bump for core:error::Error, but that's coming when we use color.

I'm also not sure how generally useful this feature would be, so feedback is welcome.

@waywardmonkeys waywardmonkeys force-pushed the support-no_std branch 2 times, most recently from 0688cd2 to 5efb018 Compare December 27, 2024 17:33
tomcur
tomcur reviewed Dec 28, 2024
View reviewed changes
Copy link
Member

@tomcur tomcur left a comment

Choose a reason for hiding this comment

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

Perhaps this is important for someone. The only real downside is the big jump in MSRV and the semver bump, so perhaps it should just be delayed until color is ready to go in as well, allowing one more patch release just prior.

@waywardmonkeys
Copy link
Collaborator Author

Yeah, I am inclined to do a 0.15.3 with roughly the current state of main and then 0.16 with color and no_std.

@waywardmonkeys waywardmonkeys added this to the 0.16 milestone Dec 28, 2024
@waywardmonkeys
Support no_std
2c630c2 
This increases the MSRV to 1.81 for `core::error`.
DJMcNab
DJMcNab approved these changes Mar 4, 2025
View reviewed changes
Copy link
Member

@DJMcNab DJMcNab left a comment

Choose a reason for hiding this comment

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

One possible tweak with how no_std is set, but otherwise looks good.

.github/workflows/ci.yml
@@ -39,6 +41,10 @@ env:
# The MSRV jobs run only cargo check because different clippy versions can disagree on goals and
# running tests introduces dev dependencies which may require a higher MSRV than the bare package.
#
# For no_std checks we target x86_64-unknown-none, because this target doesn't support std
Copy link
Member

Choose a reason for hiding this comment

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

(there are reasonable arguments to use wasm32v1-none at some point, but not for this PR - IIRC, it was only stabilised in 1.84. Cc @xStrom)

src/lib.rs
@@ -51,11 +51,14 @@
None.
*/

#![cfg_attr(all(not(feature = "std"), not(test)), no_std)]
Copy link
Member

@DJMcNab DJMcNab Mar 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think the pattern in linebender/xilem#877 is better, because of the interaction with the implicit prelude.

I will admit that I'm a little bit surprised we don't get any unused_qualification warnings as-is, but maybe they don't apply to use items.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomcur tomcur tomcur left review comments

@DJMcNab DJMcNab DJMcNab approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.16
Development

Successfully merging this pull request may close these issues.
3 participants
@waywardmonkeys @tomcur @DJMcNab