Skip to content

Document features #75

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

Closed
Kixunil opened this issue Jul 29, 2022 · 4 comments · Fixed by #88
Closed

Document features #75

Kixunil opened this issue Jul 29, 2022 · 4 comments · Fixed by #88

Comments

@Kixunil
Copy link

Kixunil commented Jul 29, 2022

I saw at least a lack of doc(cfg(...)) on std::error::Error impl there's likely more.

@tcharding
Copy link
Member

tcharding commented Mar 3, 2023

Oh, I just did #88 that adds docsrs cfg like in rust-bitcoin but while doing that I attempted to answer the question "do impls need #[cfg_attr(docsrs, doc(cfg(feature = "std")))] and came to the conclusion (by building with and without and looking at the HTML) that they did not, am I wrong?

By "impls" I mean for example

#[cfg(feature = "std")]
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
impl std::error::Error for Error {
    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
        use self::Error::*;

@Kixunil
Copy link
Author

Kixunil commented Mar 3, 2023

Are you sure you did it correctly? The last time I checked impl items do have the marker. But even if they didn't I'd argue it's a good idea to add it anyway because if it gets fixed (presumably a bug) then it'll start showing up.

@tcharding
Copy link
Member

Last time we had a discussion like this I was wrong so please leave it with me and I'll double check.

@tcharding
Copy link
Member

You were right @Kixunil, again :) I did find a useless usage though.

#[cfg(feature = "alloc")]
#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
extern crate alloc;

But the same argument you made about this changing in the future applies so will leave it in.

tcharding added a commit to tcharding/rust-bech32 that referenced this issue Mar 6, 2023
As we do in `rust-bitcoin` add a compiler conditional configuration
option `docsrs` to add feature gating to the docrs build.

Fix and improve the rustdocs while we are at it.

Add docs build to CI script but do not enable it in the github actions
yet.

Fix: rust-bitcoin#75
tcharding added a commit to tcharding/rust-bech32 that referenced this issue Mar 6, 2023
As we do in `rust-bitcoin` add a compiler conditional configuration
option `docsrs` to add feature gating to the docrs build.

Fix and improve the rustdocs while we are at it.

Add docs build to CI script but do not enable it in the github actions
yet.

Fix: rust-bitcoin#75
tcharding added a commit to tcharding/rust-bech32 that referenced this issue Mar 21, 2023
As we do in `rust-bitcoin` add a compiler conditional configuration
option `docsrs` to add feature gating to the docrs build.

Fix and improve the rustdocs while we are at it.

Add docs build to CI script but do not enable it in the github actions
yet.

Fix: rust-bitcoin#75
tcharding added a commit to tcharding/rust-bech32 that referenced this issue Mar 21, 2023
As we do in `rust-bitcoin` add a compiler conditional configuration
option `docsrs` to add feature gating to the docrs build.

Fix and improve the rustdocs while we are at it.

Add docs build to CI script but do not enable it in the github actions
yet.

Fix: rust-bitcoin#75
tcharding added a commit to tcharding/rust-bech32 that referenced this issue Apr 3, 2023
Use the recently learned about `doc_auto_cfg` feature and enable it
using a custom `docsrs` compiler conditional configuration option.

Fix and improve the rustdocs while we are at it.

Add docs build to CI script but do not enable it in the github actions
yet.

Fix: rust-bitcoin#75
tcharding added a commit to tcharding/rust-bech32 that referenced this issue Apr 3, 2023
Use the recently learned about `doc_auto_cfg` feature and enable it
using a custom `docsrs` compiler conditional configuration option.

Fix and improve the rustdocs while we are at it.

Add docs build to CI script but do not enable it in the github actions
yet.

Fix: rust-bitcoin#75
tcharding added a commit to tcharding/rust-bech32 that referenced this issue Apr 3, 2023
Use the recently learned about `doc_auto_cfg` feature and enable it
using a custom `docsrs` compiler conditional configuration option.

Fix and improve the rustdocs while we are at it.

Add docs build to CI script but do not enable it in the github actions
yet.

Fix: rust-bitcoin#75
tcharding added a commit to tcharding/rust-bech32 that referenced this issue Apr 3, 2023
Use the recently learned about `doc_auto_cfg` feature and enable it
using a custom `docsrs` compiler conditional configuration option.

Fix and improve the rustdocs while we are at it.

Add docs build to CI script but do not enable it in the github actions
yet.

Fix: rust-bitcoin#75
tcharding added a commit to tcharding/rust-bech32 that referenced this issue Apr 3, 2023
Use the recently learned about `doc_auto_cfg` feature and enable it
using a custom `docsrs` compiler conditional configuration option.

Fix and improve the rustdocs while we are at it.

Add docs build to CI script but do not enable it in the github actions
yet.

Fix: rust-bitcoin#75
apoelstra added a commit that referenced this issue Apr 3, 2023
c691a83 Add githooks directory (Tobin C. Harding)
d4191cc Improve CI pipeline (Tobin C. Harding)
ef35d89 Enable build of docs with doc_auto_cfg (Tobin C. Harding)
a37819b Remove default lint config options (Tobin C. Harding)
81b8201 Remove allow(bare_trait_objects) (Tobin C. Harding)
7fb1755 Remove allow(unknown_lints) (Tobin C. Harding)
f5cd60e Bump MSRV to Rust 1.48.0 (Tobin C. Harding)
5962169 Do trivial cleanup to readme (Tobin C. Harding)
ded35fe Use SPDX license ID (Tobin C. Harding)

Pull request description:

  Improve various things around the crate (infrastructure for want of a better word)

  **Please note:** Includes bump of MSRV to 1.48.0 because arrayvec was failing with 1.41 after improving CI.

  EDIT: Oooooh, now I get it, arrayvec feature breaks the MSRV, regardless of if its 1.41 or 1.48 - I forgot that. I"m totally sick of rebasing this so I'm leaving the MSRV bump in there.

  - CI
  - github actions
  - linting
  - docs
  - githooks

  Patch 7 `ef35d89 Enable build of docs with doc_auto_cfg` fixes #75

ACKs for top commit:
  apoelstra:
    ACK c691a83

Tree-SHA512: 6b9f684c72e72972c8ba319bd8f2337acac3563418a0f25f9f99abbc58107c0fbd8662d8afd41ee6adec9cb81996ba439f693e12c9d61e139315261d4ce48056
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 a pull request may close this issue.

2 participants