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

no_std Compatibility #30

Merged
merged 18 commits into from
Nov 6, 2020
Merged

no_std Compatibility #30

merged 18 commits into from
Nov 6, 2020

Conversation

mriise
Copy link
Contributor

@mriise mriise commented Nov 2, 2020

Waiting for #27 for me to keep sanity over changes.

fixes #24 thanks to work done by @whalelephant and @ia0 over in #25

This allows multibase to only require a global allocator in no_std environments.

@mriise mriise changed the title No std no_std Compatibility Nov 2, 2020
@vmx
Copy link
Member

vmx commented Nov 2, 2020

I just merged #27. BTW, you want to mention @whalelephant instead :)

@mriise
Copy link
Contributor Author

mriise commented Nov 2, 2020

ah thanks, took glasses off for a minute and didnt bother to read the whole thing...

@mriise mriise marked this pull request as ready for review November 2, 2020 18:13
@mriise mriise requested a review from vmx November 2, 2020 18:13
@mriise
Copy link
Contributor Author

mriise commented Nov 2, 2020

@vmx it might be good to add some info in README and adding 'no_std' to the cargo tags- things like that before merge.

Copy link
Member

@vmx vmx left a comment

Choose a reason for hiding this comment

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

I make this a "request changes" due to the git based dependency of the base-x-rs fork.

@mriise feel free to add the "no_std" tag as part of this PR.

Cargo.toml Outdated Show resolved Hide resolved
.cargo/config.toml Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
@mriise
Copy link
Contributor Author

mriise commented Nov 6, 2020

@vmx after this PR could we get a no_std build target added to CI?

README.md Outdated Show resolved Hide resolved
@vmx vmx self-requested a review November 6, 2020 10:15
@vmx
Copy link
Member

vmx commented Nov 6, 2020

@vmx after this PR could we get a no_std build target added to CI?

Yes we should. I tried things locally and I think we should use the cargo build --target thumbv6m-none-eabi --no-default-feature trick (https://blog.dbrgn.ch/2019/12/24/testing-for-no-std-compatibility/). For me cargo nono doesn't report it properly, it even doesn't return an error if I compile with the std feature enabled.

@mriise
Copy link
Contributor Author

mriise commented Nov 6, 2020

Like this?

Copy link
Member

@vmx vmx left a comment

Choose a reason for hiding this comment

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

The rest looks good!

.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
@mriise
Copy link
Contributor Author

mriise commented Nov 6, 2020

There we go

@mriise mriise requested a review from vmx November 6, 2020 11:48
Copy link
Member

@vmx vmx 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 hard work and fighting CI!

@vmx vmx merged commit e5d1780 into multiformats:master Nov 6, 2020
@mriise mriise deleted the no_std branch November 6, 2020 12:08
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.

Any plans to support no_std?
2 participants