rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Yury Norov <yury.norov@gmail.com>
To: Alice Ryhl <aliceryhl@google.com>
Cc: "Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com>,
	"Miguel Ojeda" <ojeda@kernel.org>, "Burak Emir" <bqe@google.com>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
	patches@lists.linux.dev
Subject: Re: [PATCH] rust: id_pool: fix example
Date: Tue, 2 Dec 2025 13:33:04 -0500	[thread overview]
Message-ID: <aS8w4FHhIVQUFTEY@yury> (raw)
In-Reply-To: <aS7aHIJbi3DlaUcc@google.com>

On Tue, Dec 02, 2025 at 12:22:52PM +0000, Alice Ryhl wrote:
> On Tue, Dec 02, 2025 at 12:13:33AM +0100, Miguel Ojeda wrote:
> > On Mon, Dec 1, 2025 at 8:29 PM Yury Norov <yury.norov@gmail.com> wrote:
> > >
> > > Thanks Miguel,
> > >
> > > I applied this, but the fact that you've sent a second fix to
> > > documentation that actually is a build fix, raises the questions.
> > >
> > > Because Rust documentation bears compilable chunks of code, I think
> > > we need to enable rustdoc tests target by default, so that developers
> > > will not send broken tests.
> > 
> > You're welcome!
> > 
> > This one is a Kconfig in the normal build, i.e.
> > `CONFIG_RUST_KERNEL_DOCTESTS` (it is true that the other patch was for
> > a different target).
> > 
> > If you mean enabling that by Kconfig default, then I don't think we
> > can do that -- KUnit on its own (which this uses) is not meant for
> > normal builds. Perhaps we could have something that just checks the
> > build, but people should really run the doctests anyway, since they
> > typically contain `assert!`s and so on that can be wrong.
> > 
> > If you mean enabling it in Intel's kernel test robot, then I think
> > they do it (or at least I told them about it long ago). But maybe
> > something is missing.
> > 
> > In our P entry in `MAINTAINERS` ("Subsystem Profile document") we ask
> > people to run them among other things, so I would perhaps suggest
> > having something like that too (or linking to ours if you prefer):
> > 
> >     https://rust-for-linux.com/contributing#submit-checklist-addendum
> > 
> > Of course new contributors will miss that initially, but actually
> > people find the doctests quite useful, so generally people get to run
> > them. At the end of the day, maintainers should test these too before
> > applying and otherwise someone will notice sooner or later.

No. Maintainers are not handy testers at your convenience.
 
> Yeah sorry I forgot to check the docs this time. I usually do run them,
> but there are so many targets to check that sometimes I forget some of
> them.

Don't be sorry, it's not your fault. The problem is that bearing
tests smeared among comments is a terrible idea, and now you reap
the benefits of someone else's bad design.

(Did I warn about it? Yes I did.)

The other problem is that Miguel replied in great details about how
well rust process is designed, and even attached an external link,
but didn't answer my question: how to enable rustdoc by default?

I followed this link by the way. It doesn't mention that rustdoc step
is mandatory. It doesn't provide steps for careful testing. It only
says:

        When submitting changes to Rust code documentation, please
        render them using the rustdoc target and ensure the result
        looks as expected. The Rust code documentation gets rendered
        at https://rust.docs.kernel.org.

Does it sound like:

        Rustdoc is a MANDATORY step! You MUST run rustdoc every single
        time, otherwise the build will get BROKEN. Even if you don't
        touch comments!

No, it does not. So, I wasted an hour just to find nothing. But there
are some good news: during that hour, I received an LKP email about
your error:

https://lore.kernel.org/oe-kbuild-all/202512020552.NejH5iaK-lkp@intel.com/

It's good that I started receiving such a traffic at all. But it's
especially good because I was on my way to submit a PR to Linus, and
that hour saved me a broken request. Andrew Morton on the previous
cycle wasn't that lucky, didn't he?

So, what's next. 

1. I will merge-fold Miguel's fixes into your patches and send my
   PR by the end of this week, not early the week as I usually do.
   Hopefully, no objections.

2. I will stop taking any rust patches unless the author explicitly
   mentions that he ran rustdoc on them.

3. I encourage rust community to spend this cycle on reviewing the
   development and submitting process. You guys decided to sprinkle
   compilable code across the comments, and you advocate it. Now
   please find a way for everyone to compile your comments (huh)
   during a routine development. And please make that knowledge
   very sound.

Thanks,
Yury

      reply	other threads:[~2025-12-02 18:33 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-01  0:09 [PATCH] rust: id_pool: fix example Miguel Ojeda
2025-12-01 10:07 ` Alice Ryhl
2025-12-01 19:28 ` Yury Norov
2025-12-01 23:13   ` Miguel Ojeda
2025-12-02 12:22     ` Alice Ryhl
2025-12-02 18:33       ` Yury Norov [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aS8w4FHhIVQUFTEY@yury \
    --to=yury.norov@gmail.com \
    --cc=a.hindborg@kernel.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=bqe@google.com \
    --cc=dakr@kernel.org \
    --cc=gary@garyguo.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=ojeda@kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).