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

Using no_std + alloc #279

Merged
merged 5 commits into from
Jul 30, 2021
Merged

Using no_std + alloc #279

merged 5 commits into from
Jul 30, 2021

Conversation

halzy
Copy link
Contributor

@halzy halzy commented Jul 19, 2021

Removed references to std and replaced them with references to core and alloc.

Tests still use std.

Copy link
Owner

@lambda-fairy lambda-fairy left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

Happy to merge this once the issues above are resolved.

BTW, if you'd like a quicker turnaround next time, I recommend running cargo test --workspace --all-features locally before sending the PR. That should catch most of these issues.

CHANGELOG.md Outdated Show resolved Hide resolved
maud/src/lib.rs Outdated
Comment on lines 12 to 15
// So that unit tests can use `std`
#[cfg(test)]
#[macro_use]
extern crate std;
Copy link
Owner

Choose a reason for hiding this comment

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

Can we remove this? This module doesn't define any unit tests afaik.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was updated to re-enable std for rocket and iron support.

maud/src/lib.rs Outdated
@@ -168,11 +179,11 @@ pub const DOCTYPE: PreEscaped<&'static str> = PreEscaped("<!DOCTYPE html>");
#[cfg(feature = "iron")]
mod iron_support {
use crate::PreEscaped;
use core::io;
Copy link
Owner

Choose a reason for hiding this comment

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

Does this work?

I'm pretty sure core doesn't have an io module.

You might want to disable no_std when these web framework features are enabled, since they require std anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't find a way to optionally disable no_std, only a way to optionally enable it.

maud_macros/src/lib.rs Outdated Show resolved Hide resolved
@lambda-fairy lambda-fairy changed the title Using no_std + alloc. Using no_std + alloc Jul 25, 2021
@lambda-fairy lambda-fairy linked an issue Jul 25, 2021 that may be closed by this pull request
Removed references to `std` and replaced them with references to `core` and `alloc`.
Features `rocket` and `iron` still require `std`.
@halzy halzy force-pushed the halzy/no_std branch 2 times, most recently from 588c22f to 1c0129a Compare July 25, 2021 16:29
@halzy
Copy link
Contributor Author

halzy commented Jul 25, 2021

BTW, if you'd like a quicker turnaround next time, I recommend running cargo test --workspace --all-features locally before sending the PR. That should catch most of these issues.

Ah, this would have been helpful. I'll open a second PR for updating the CONTRIBUTING.md file with this information.

@halzy
Copy link
Contributor Author

halzy commented Jul 29, 2021

@lambda-fairy CI should pass. Let me know if there are other changes you'd like.

Copy link
Owner

@lambda-fairy lambda-fairy left a comment

Choose a reason for hiding this comment

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

LGTM (with a couple minor fixes I made myself)

Thanks again @halzy!

@lambda-fairy lambda-fairy merged commit 3eb4254 into lambda-fairy:master Jul 30, 2021
@lambda-fairy
Copy link
Owner

Oh, I notice GitHub requires maintainer approval before it runs CI. I've changed it to run automatically instead.

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.

no_std support
2 participants