docs: Add doc comments for external crates

[yancyribbens]

A new version of rust nightly is causing a test failure for external crates that are not documented. This is currently causing a failure on my branch here: #1838

As a side note, it feels like this should be locked to a specific version of nightly in the CI tests and then updated as a choir instead of having it create a new problem in a branch that's not related.

[apoelstra]

Looks great! Thanks for doing this so quickly (and for doing it properly :P I'd have accepted a bunch of "re-export of X" type comments because why is it our job to document re-exported crates..)

[apoelstra]

ACK 639c548

[tcharding]

ACK 639c548

[tcharding]

No need to change it but typically we put rustdocs above the attributes.

[tcharding]

Thanks @yancyribbens!

[sanket1729]

Merging this to fix red cross CI on master.

[Kixunil]

As a side note, it feels like this should be locked to a specific version of nightly in the CI tests and then updated as a choir instead of having it create a new problem in a branch that's not related.

Actually we have this on intentionally to catch potential nightly bugs. But ideally it shouldn't be mandatory.

Maybe we should switch it to cron and just run it once every day instead.

[yancyribbens]

Maybe we should switch it to cron and just run it once every day instead.

Maybe. There was someone asking about why their branch was failing in IRC yesterday also. I don't know if there's a way to be notified when there's a new nightly build available, but if there is, we could just manually update the nightly version then in CI?

[Kixunil]

Nightly is called such because it's literally released at midnight. :) Manual updating would add a lot of churn, I definitely don't want that.

[apoelstra]

Actually we have this on intentionally to catch potential nightly bugs. But ideally it shouldn't be mandatory.

FWIW I download the latest nightly every day and test PRs with that, so we have at-least local testing from my end.

But personally I don't think it's too big a deal to just fix nightly failures when they crop up. Yes, we get a red X for a day or two, but it's easy to check and ignore, and it's not super frequent.

[tcharding]

I'm confused as to why this issue came up, we do not run clippy with nightly toolchain in CI? What am I missing?

[tcharding]

oooo, its from cargo +nightly doc --all-features

[apoelstra]

Yeah, and we have -D warnings on that ... which I think is fine, but is certainly debateable.

[Kixunil]

-D warnings is there because we want to catch broken links and we need to build with nightly because we want to document features. And I don't want to pin the nightly used for documenting because if something changes the docs.rs doc may get broken which would be very bad.

[yancyribbens]

But personally I don't think it's too big a deal to just fix nightly failures when they crop up. Yes, we get a red X for a day or two, but it's easy to check and ignore, and it's not super frequent.

Personally I don't mind fixing these issues either when they crop up. It just feels like unrelated work to fix the warning that has nothing to do with the PR. A cron job or something seems like a good idea; the down side is it's more infrastructure to setup and maintain and then someone needs to watch for the issues I guess.