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

fix build #17

Closed
wants to merge 1 commit into from
Closed

fix build #17

wants to merge 1 commit into from

Conversation

zoedberg
Copy link
Contributor

As reported in RGB-WG/rgb-node#224 (comment) documentation of lnp-core doesn't build on docs.rs (https://docs.rs/crate/lnp-core/0.8.0/builds/592909).

Cloning the repository locally and running cargo build produces the same error logs we see on docs.rs. Adding bifrost to the default features of lnp2p seems to fix the build.

@dr-orlovsky if you think this fix is appropriate then you can release this first and rgb-node after. Thank you

@codecov-commenter
Copy link

Codecov Report

Merging #17 (6b58060) into master (9e235c0) will decrease coverage by 3.3%.
The diff coverage is 26.5%.

@@           Coverage Diff            @@
##           master     #17     +/-   ##
========================================
- Coverage    37.7%   34.4%   -3.3%     
========================================
  Files          20      17      -3     
  Lines        1757    1883    +126     
========================================
- Hits          662     647     -15     
- Misses       1095    1236    +141     
Impacted Files Coverage Δ
lnp2p/src/bifrost/app.rs 0.0% <0.0%> (ø)
lnp2p/src/bifrost/channel.rs 0.0% <0.0%> (ø)
lnp2p/src/bifrost/msg.rs 0.0% <0.0%> (ø)
lnp2p/src/bifrost/proposals.rs 0.0% <0.0%> (ø)
lnp2p/src/bolt/bolt1.rs 50.0% <0.0%> (ø)
lnp2p/src/bolt/bolt11.rs 0.0% <0.0%> (ø)
lnp2p/src/bolt/bolt2.rs 10.6% <0.0%> (ø)
lnp2p/src/bolt/bolt4.rs 0.0% <0.0%> (ø)
lnp2p/src/bolt/bolt7.rs 0.0% <0.0%> (ø)
lnp2p/src/bolt/bolt9.rs 52.7% <0.0%> (ø)
... and 9 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@dr-orlovsky
Copy link
Contributor

It seems that the problem was caused by releasing the library without --all-features. What you propose is to enable all features by default, which is incorrect (it is ok for the crate to have a non-default features; a lot of such crates build on docs.rs). I will try to do a release with --all-features and nothing else changed - lets see would it help. Another option is to add

[package.metadata.docs.rs]
features = ["all"]

to the Cargo.toml, which I think will be a good solution for all our crates

@dr-orlovsky
Copy link
Contributor

Closing in favor of e25d73d

@dr-orlovsky dr-orlovsky added this to the v0.9.0 milestone Jan 18, 2023
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