Skip to content

Removed support for no_std #109

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

Merged
merged 7 commits into from
Apr 21, 2022
Merged

Removed support for no_std #109

merged 7 commits into from
Apr 21, 2022

Conversation

esteve
Copy link
Collaborator

@esteve esteve commented Apr 19, 2022

This PR removes support for no_std, by removing the dependency for the spin crate and the corresponding cfg() blocks.

I recall @nnmm saying that no_std is broken (#88 (comment)) and I thought we might as well just remove support for it and revisit this in the future if we want to.

@esteve esteve requested review from jhdcs and nnmm April 19, 2022 10:09
@esteve
Copy link
Collaborator Author

esteve commented Apr 19, 2022

Kudos to @jhdcs for implementing support for no_std in the first place.

@esteve esteve force-pushed the remove-no_std branch 3 times, most recently from b66213c to 6cfa7a6 Compare April 19, 2022 11:00
jhdcs
jhdcs previously approved these changes Apr 19, 2022
Copy link
Collaborator

@jhdcs jhdcs left a comment

Choose a reason for hiding this comment

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

Too bad things aren't working, but if we ever decide to re-add it, I hope that what I did could serve as a template.

@nnmm
Copy link
Contributor

nnmm commented Apr 19, 2022

I think so, if we want to add it back we know how to do that.

@@ -613,7 +611,7 @@ impl ToResult for rcl_ret_t {

#[cfg(test)]
mod tests {
use core::convert::TryFrom;
use std::convert::TryFrom;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should not be needed iirc, since we're on the 2021 Rust edition

nnmm
nnmm previously approved these changes Apr 19, 2022
jhdcs
jhdcs previously approved these changes Apr 20, 2022
fmt::{self, Display},
};
use core_error::{self, Error};
use std::convert::TryFrom;
Copy link
Collaborator

Choose a reason for hiding this comment

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

If @nnmm is correct about us not needing this for Rust 2021, then this line can be removed as well.

@esteve esteve dismissed stale reviews from jhdcs and nnmm via dee56d5 April 21, 2022 13:12
nnmm
nnmm previously approved these changes Apr 21, 2022
Copy link
Contributor

@nnmm nnmm 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 making the code cleaner :)

nnmm
nnmm previously approved these changes Apr 21, 2022
@esteve esteve merged commit 5d4f613 into master Apr 21, 2022
@esteve esteve deleted the remove-no_std branch April 21, 2022 18:55
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