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] should be declared in binary crate root instead of library. . #4

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

Conversation

krystian-wojtas
Copy link

…Otherwise it cannot be used by std projects

When I try to use si7021 master in another project without this patch, I get following build errors:
https://gitlab.com/krystian.wojtas/weather_station/-/jobs/438899063

@krystian-wojtas krystian-wojtas changed the title [no_std] should be declared in binary crate root instead of library. … [no_std] should be declared in binary crate root instead of library. . Feb 16, 2020
@krystian-wojtas
Copy link
Author

Here is build with patch applied
https://gitlab.com/krystian.wojtas/weather_station/-/jobs/438901509

@krystian-wojtas
Copy link
Author

Pipeline in your orginal project failed because my email address was not verified in github. I don't know how to rerun it now.

@lnicola
Copy link

lnicola commented Feb 16, 2020

You seem to have vendored (included) the code from this crate into your project. That's why you're getting unused code warnings. Is there a reason you're not using it via cargo instead?

To be more clear, your changes would be fine for your own project, but I don't see a reason to apply them here.

@lnicola lnicola mentioned this pull request Feb 16, 2020
@wose
Copy link
Owner

wose commented Feb 16, 2020

Hi,

basically what @lnicola said. Are you aware that you can add unreleased crates to your project by telling cargo the git uri? Something like:

[dependencies]
si7021 = { git = "https://github.com/wose/si7021" }

@lnicola
Copy link

lnicola commented Feb 16, 2020

@wose I'm on my phone so I haven't checked, but would you consider publishing it? There's quite a bit of squatting going on, and I think someone already wrote another SHT1x driver, probably because they weren't aware of this one.

@wose
Copy link
Owner

wose commented Feb 16, 2020

@lnicola Yes. I wanted to publish the crate during last week after adding some more documentation, but didn't got around to finish it. It will probably take another few days. I also looked into supporting some similar sensors with this crate. I thought about renaming the crate it to something like si70xx before publishing it to crates.io.

@krystian-wojtas
Copy link
Author

I was not aware that git links works as cargo dependecies without pulishing to crates.io. Thanks for that. I will do it this way.

@krystian-wojtas
Copy link
Author

@wose I have si7021 as git link as you adviced me.
https://gitlab.com/krystian.wojtas/weather_station/-/tree/dev-si7021-crate-from-git

But I cannot publish new release with git link

% cargo publish
    Updating crates.io index
error: all dependencies must have a version specified when publishing.
dependency `si7021` does not specify a version
Note: The published dependency will use the version from crates.io,
the `git` specification will be removed from the dependency declaration

@lnicola

This comment has been minimized.

@krystian-wojtas
Copy link
Author

Ok, it should be easy
rust-lang/cargo#126

@lnicola
Copy link

lnicola commented Feb 16, 2020

Ah, sorry, I misread your comment. Indeed, you can't publish a crate with git dependencies, not even if you specify the commit (I think).

@krystian-wojtas
Copy link
Author

krystian-wojtas commented Feb 16, 2020

Right, exact commit didn't help. Still same error message

% cargo publish
    Updating crates.io index
error: all dependencies must have a version specified when publishing.
dependency `si7021` does not specify a version
Note: The published dependency will use the version from crates.io,
the `git` specification will be removed from the dependency declaration.

https://gitlab.com/krystian.wojtas/weather_station/-/commit/74eb953f813055ae1d433204f2dce9ff55b9c00c

@krystian-wojtas
Copy link
Author

For clarification as temporarily workaround external si7021 source is moved to src/external directory and mentioned in README

It's in master
https://gitlab.com/krystian.wojtas/weather_station

@krystian-wojtas
Copy link
Author

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