Skip to content

Use nightly feature documentation #6630

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
wants to merge 2 commits into from

Conversation

robertbastian
Copy link
Member

@robertbastian robertbastian commented May 23, 2025

Instead of documenting "✨ Enabled with the xy Cargo feature.", this uses the rustdoc doc_cfg feature. These automatically apply recursively to items within modules as well.

image image

Copy link

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

@@ -88,6 +88,7 @@
)
)]
#![warn(missing_docs)]
#![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why only docsrs?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It needs nightly and that's the cfg that docs.rs uses. No point introducing a second one.

command = "cargo"
toolchain = "${PINNED_CI_NIGHTLY}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thought: I wish this didn't need nightly. But it is a really nice improvement

@robertbastian robertbastian requested a review from sffc May 23, 2025 15:57
Copy link
Member

@Manishearth Manishearth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

autocfg is a long awaited feature, unfortunately the reason it's taken so long is that it's not well designed and nobody wanted to stabilize it as is. There's a recently-merged RFC to fix it, and it stabilization shouldn't be too far off into the future, but given that it is basically guaranteed that this will stabilize in an incompatible way, I'd rather not have us use this for docs.rs and at least wait for the new RFCd feature to be implemented even if it hasn't stabilized yet.

There has been talk of occasionally regenerating docs on docs.rs with newer compilers so that crates get newer rustdoc features. That's partly why I'm hesitant to use nightly features for docs, especially ones we know are going to change.

I also personally find myself using local docs often enough for our dependencies and having these banners unavailable to them is suboptimal.

I think doc(cfg) itself (not doc_auto_cfg) is something I could be convinced towards us using: That part of the feature is expected to be unchanged, see the [accepted RFC](https://github.com/rust-lang/rfcs/blob/master/text/3631-rustdoc-cfgs-handling.md#guide-level-explanation. That feature alone is quite valuable to have since it lets you do the module virality thing. It's still unstable, though, it might be worth it to just wait for stabilization. Rustdoc doesn't tend to take long for that once things are RFCd.

Curious on @sffc's thoughts.

@Manishearth
Copy link
Member

Manishearth commented May 23, 2025

With the expectation that this will stabilize soon I'm happy to just hold off on using it until that point, too, and then enabling it projectwide. The downside is we don't get to use module-virality for #6629.

A more scoped change would be to use cfg_attr(docsrs, doc(cfg(feature = "unstable"))) on provider and unstable modules for the purpose of #6629 only, leaving the rest of our ✨ headers around. It's inconsistent, but that's fine the unstable feature is strange anyway. Still uses unstable features, but like I said I can be convinced towards us using just this "unlikely to change" unstable feature.

@sffc sffc removed their request for review May 23, 2025 21:13
@sffc
Copy link
Member

sffc commented May 23, 2025

I think I'm mildly in favor of using nightly rustdoc if it makes things nicer (assuming it works on docs.rs) but I'm also happy sticking with the status quo. I defer to @Manishearth

@Manishearth
Copy link
Member

I think my position is:

  • We shouldn't use a nightly docs feature we know will change incompatibly (so no autocfg)
  • I'd prefer we didn't use nightly features at all, but could be swayed otherwise
  • Without autocfg this replaces one bit of manual code with another bit of manual code; which is not that much of a win IMO. I'd rather wait for this to hit stable Rust.
  • I'd rather not have to cfg(docsrs) so everyone benefits.
  • I do not care at all about MSRV for docsrs stuff: to me it's okay to start using this feature the moment it hits stable.

@robertbastian
Copy link
Member Author

We shouldn't use a nightly docs feature we know will change incompatibly (so no autocfg)

How will autocfg change? Currently I just enable a feature, there's not API or anthing that might change.

Without autocfg this replaces one bit of manual code with another bit of manual code; which is not that much of a win IMO. I'd rather wait for this to hit stable Rust.

I think it's a big win, because of the way that rustdoc renders these.

I'd rather not have to cfg(docsrs) so everyone benefits.

We need some kind of cfg, because we cannot do cfg(nightly). We might as well use docsrs, because that's what docsrs does.

@Manishearth
Copy link
Member

How will autocfg change? Currently I just enable a feature, there's not API or anthing that might change.

The syntax will change; it becomes #[doc(auto_cfg)].

Though it looks like it's enabled by default (and will continue to be enabled by default), so that's not a big deal I guess.

In that case I'm fine with adding cfg(docsrs, feature(...)) but we should not remove the existing banners until the feature hits latest stable.

We need some kind of cfg, because we cannot do cfg(nightly). We might as well use docsrs, because that's what docsrs does.

No, we do not need some kind of cfg; the status quo works, though it's suboptimal. You only need a cfg if you're using a nightly feature. My proposal is we wait until it's not unstable anymore.

@robertbastian
Copy link
Member Author

shame

@Manishearth
Copy link
Member

(I am going to see what's left to get this stabilized because I don't think it's much; it's minor impl work)

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