Clean up doc links and enforce them in CI #848

TheBlueMatt · 2021-03-17T19:57:30Z

Also adds some compile-time enforcement of doc links and missing doc fixes.

codecov · 2021-03-17T20:21:23Z

Codecov Report

Merging #848 (e447131) into main (32f6205) will decrease coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #848      +/-   ##
==========================================
- Coverage   90.66%   90.63%   -0.04%     
==========================================
  Files          51       51              
  Lines       27135    27135              
==========================================
- Hits        24603    24593      -10     
- Misses       2532     2542      +10

Impacted Files	Coverage Δ
background-processor/src/lib.rs	`100.00% <ø> (ø)`
lightning-block-sync/src/http.rs	`95.39% <ø> (ø)`
lightning-block-sync/src/init.rs	`93.71% <ø> (ø)`
lightning-block-sync/src/lib.rs	`95.37% <ø> (ø)`
lightning-block-sync/src/poll.rs	`92.04% <ø> (ø)`
lightning-block-sync/src/rest.rs	`67.24% <ø> (ø)`
lightning-block-sync/src/rpc.rs	`79.48% <ø> (ø)`
lightning-net-tokio/src/lib.rs	`76.42% <ø> (ø)`
lightning-persister/src/lib.rs	`95.61% <ø> (ø)`
lightning/src/chain/chainmonitor.rs	`93.05% <ø> (ø)`
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 32f6205...e447131. Read the comment docs.

jkczyz

Nice! I didn't know this feature was stable.

lightning-block-sync/src/poll.rs

lightning-persister/src/lib.rs

lightning/src/ln/channelmanager.rs

lightning-block-sync/src/init.rs

background-processor/src/lib.rs

lightning/src/chain/chainmonitor.rs

jkczyz · 2021-03-17T20:27:06Z

lightning-block-sync/src/poll.rs

@@ -1,3 +1,5 @@
+//! Adapters which make one or more [`BlockSource`]s simpler to poll for new chain tip transitions.


Will this work given the explanation for using the crate:: prefix in 1e71d32? Likewise in other files.

As long as its imported in a use statement later in the file it seems to work (cargo doc doesn't error, which it now will if it doesnt resolve).

lightning-block-sync/src/rpc.rs

jkczyz · 2021-03-17T20:29:31Z

lightning/src/chain/channelmonitor.rs

@@ -1187,8 +1181,6 @@ impl<Signer: Sign> ChannelMonitor<Signer> {

 	/// Get the list of HTLCs who's status has been updated on chain. This should be called by
 	/// ChannelManager via [`chain::Watch::release_pending_monitor_events`].


Are parentheses needed here or are they optional? I noticed that you add them elsewhere.

I believe parentheses are only required when the link is ambiguous in some way. cargo doc fails with an ambiguity error in that case.

TheBlueMatt · 2021-03-17T21:34:41Z

I didn't know this feature was stable.

Yea, I think its recent-ish. Certainly newer than a number of the html links in our codebase.

valentinewallace

Compile-time enforcement == happiness

lightning/src/ln/channelmanager.rs

lightning-block-sync/src/rest.rs

jkczyz

LGTM but prefer to drop ticks from the right-hand side as noted.

lightning/src/ln/channelmanager.rs

background-processor/src/lib.rs

TheBlueMatt · 2021-03-18T00:31:22Z

Addressed comments, happy to squash if @valentinewallace is happy with the changes.

jkczyz · 2021-03-18T01:04:41Z

Not sure if it can be fixed, but FWIW older versions of Rust are giving the following warning:

warning: unknown lint: `broken_intra_doc_links`
  --> lightning/src/lib.rs:23:9
   |
23 | #![deny(broken_intra_doc_links)]
   |         ^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: #[warn(unknown_lints)] on by default

TheBlueMatt · 2021-03-18T01:06:41Z

Yea, I'm not aware of a (dependency-free) compile-time change to gate that on the rustc version. Not a huge deal, anyway.

Relative HTML doc paths in doc links works locally, but breaks on crates.io. Luckily, we can now use explicit full paths and rustdoc will resolve them for us.

TheBlueMatt · 2021-03-18T15:29:11Z

Squashed fixup commits with no changes:

$ git diff-tree -U5 529af8e8 e44713190a090156e4bffac518dad98574e411be
$

TheBlueMatt force-pushed the 2021-03-doc-cleanups branch from 3d96413 to 1a6becc Compare March 17, 2021 20:05

TheBlueMatt mentioned this pull request Mar 17, 2021

Make cltv_expiry_delta configurable and reduce the min/default some #849

Merged

jkczyz reviewed Mar 17, 2021

View reviewed changes

TheBlueMatt force-pushed the 2021-03-doc-cleanups branch from 1a6becc to 0516b20 Compare March 17, 2021 21:33

valentinewallace approved these changes Mar 17, 2021

View reviewed changes

lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved

lightning-block-sync/src/rest.rs Outdated Show resolved Hide resolved

TheBlueMatt force-pushed the 2021-03-doc-cleanups branch from 0516b20 to 3b81049 Compare March 17, 2021 22:07

jkczyz approved these changes Mar 17, 2021

View reviewed changes

lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved

background-processor/src/lib.rs Outdated Show resolved Hide resolved

TheBlueMatt force-pushed the 2021-03-doc-cleanups branch from 3b81049 to 529af8e Compare March 18, 2021 00:30

valentinewallace approved these changes Mar 18, 2021

View reviewed changes

TheBlueMatt added 6 commits March 18, 2021 11:28

Expose the constants mentioned in docs, and use doc links

c88e3eb

Clean up some doc links in lightning_block_sync.

98c6566

Relative HTML doc paths in doc links works locally, but breaks on crates.io. Luckily, we can now use explicit full paths and rustdoc will resolve them for us.

Enforce doc link correctness during cargo doc runs and in CI

18a0967

Enforce no missing docs in all crates (+ add docs that were missing)

2c7c1a6

Drop all HTML-relative links since rustdoc now supports resolution

836985a

Also correct/check links in private item documentation

e447131

TheBlueMatt force-pushed the 2021-03-doc-cleanups branch from 529af8e to e447131 Compare March 18, 2021 15:28

TheBlueMatt merged commit fba204b into lightningdevkit:main Mar 18, 2021

		@@ -1,3 +1,5 @@
		//! Adapters which make one or more [`BlockSource`]s simpler to poll for new chain tip transitions.

		@@ -1187,8 +1181,6 @@ impl<Signer: Sign> ChannelMonitor<Signer> {

		/// Get the list of HTLCs who's status has been updated on chain. This should be called by
		/// ChannelManager via [`chain::Watch::release_pending_monitor_events`].

Clean up doc links and enforce them in CI #848

Clean up doc links and enforce them in CI #848

Uh oh!

Conversation

TheBlueMatt commented Mar 17, 2021

Uh oh!

codecov bot commented Mar 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jkczyz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jkczyz Mar 17, 2021

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt Mar 17, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jkczyz Mar 17, 2021

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt Mar 17, 2021

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt commented Mar 17, 2021

Uh oh!

valentinewallace left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jkczyz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

TheBlueMatt commented Mar 18, 2021

Uh oh!

jkczyz commented Mar 18, 2021

Uh oh!

TheBlueMatt commented Mar 18, 2021

Uh oh!

TheBlueMatt commented Mar 18, 2021

Uh oh!

Uh oh!

codecov bot commented Mar 17, 2021 •

edited

Loading

valentinewallace left a comment •

edited

Loading