Fix rust beta docs for lightning-invoice crate. #1490
Conversation
|
Do we not need/can we add the |
arik-so
force-pushed
the
rust_beta_doc_fix
branch
from
2f79bf9 to
c8a3e2e
Compare
Codecov Report
@@ Coverage Diff @@
## main #1490 +/- ##
==========================================
- Coverage 90.90% 90.90% -0.01%
==========================================
Files 77 77
Lines 42201 42201
Branches 42201 42201
==========================================
- Hits 38364 38362 -2
- Misses 3837 3839 +2
Continue to review full report at Codecov.
|
arik-so
force-pushed
the
rust_beta_doc_fix
branch
from
c8a3e2e to
dac596d
Compare
|
Oof, looks like we need it to use defined types....matbe lets just not make it a code doc? |
I think |
|
Oh lol, I have already started figuring out how to fix this |
|
Oh, and |
lightning-invoice/src/lib.rs
Outdated
| @@ -796,17 +796,18 @@ impl SignedRawInvoice { | |||
| /// | |||
| /// The following example would extract the first B. | |||
| /// ``` | |||
| /// use Enum::*; | |||
| /// use lightning_invoice::{find_extract, find_all_extract}; | |||
There was a problem hiding this comment.
nit: only need to import one of them here and below.
There was a problem hiding this comment.
not technically true. Both need to be imported here, but find_all_extract is indeed sufficient below.
lightning-invoice/src/lib.rs
Outdated
| /// ``` | ||
| #[macro_export] |
There was a problem hiding this comment.
Not sure if we want to export these though... Is there a way to do it conditionally for tests?
There was a problem hiding this comment.
I think we just need to make it a non-``` comment so that rustc doesn't try to run it.
There was a problem hiding this comment.
Tried #[cfg_attr(test, macro_export)], but that's not working. It'd be a shame to make the code not be evaluated, as it's verifying the accuracy of the comment though.
There was a problem hiding this comment.
It's even wilder. When I explicitly say #[cfg_attr(not(test), macro_export)] as a sanity check, it works!
There was a problem hiding this comment.
There was a problem hiding this comment.
Ah, you've found the attribute Google wouldn't show me. Yet alas, also no.
There was a problem hiding this comment.
So question. If the docs show how to use the macro, wouldn't that require the macro be public? Otherwise the docs are simply inaccurate.
There was a problem hiding this comment.
I think it was more to test that the macros works as expected and to show internal developers how to use it. It's possible to generate docs for private items, too.
Suppose it could have be a normal test. Maybe already covered by other tests, though. I'd say just remove the ticks as Matt suggested.
There was a problem hiding this comment.
Ok, I gave up on the macros and reverted everything except the semicolons.
arik-so
force-pushed
the
rust_beta_doc_fix
branch
from
f500711 to
f9d3576
Compare
lightning-invoice/src/lib.rs
Outdated
| /// assert_eq!(find_extract!(elements.iter(), Enum::B(ref x), x), Some(3u16)) | ||
| /// ``` | ||
| /// assert_eq!(find_extract!(elements.iter(), Enum::B(ref x), x), Some(3u16)); | ||
| /// `` |
There was a problem hiding this comment.
Does double ticking do anything? Thought that would just be the empty string formatted in markdown.
lightning-invoice/src/lib.rs
Outdated
| /// ``` | ||
| /// use Enum::* | ||
| /// `` | ||
| /// use Enum::*; |
There was a problem hiding this comment.
May as well have kept the fully-qualified enum variants as I don't think this was ever valid rust. 😛
arik-so
force-pushed
the
rust_beta_doc_fix
branch
from
f9d3576 to
7182287
Compare
arik-so
force-pushed
the
rust_beta_doc_fix
branch
from
7182287 to
71d89d2
Compare
No description provided.