| Message ID | 20240909161749.147076-1-hridesh699@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | docs: rust: clean up empty `\\\` lines and improve rustdoc formatting | expand |
Hi, I see that this is your first kernel contribution, welcome! I have left a couple comments below; before you send a new version, please wait a couple days for other people to also leave their feedback. You then create a new version (add `-v2` to `git format-patch`) and send it to the list. You can put a changelog underneath the `---`, it will not be included int the commit message, but for people reading the mail it is rather helpful. On 09.09.24 18:17, hridesh wrote: > Remove unnecessary empty `\\\` lines in the rust docs. Also add linebreaks You wrote backslashes here, but it should be forward slashes instead. Please also fix it in the title. I don't know if the commit title should start with `docs`, maybe we want to do `rust: docs` when changing rustdocs? (This is a question to the other Rust reviewers) I think the title doesn't need to mention the exact cleanup, just something along the lines "clean up docs" should suffice. > in kernel::block::mq::Request to fix formatting > > Suggested-by: Miguel Ojeda <ojeda@kernel.org> > Link: https://github.com/Rust-for-Linux/linux/issues/1109 The issue also mentions that you should implement a `checkpatch.pl` check in an additional patch: > Clean up consecutive empty `///` lines and implement a checkpatch.pl > check for it. These should be two different patches. Please include that patch in your series. > Signed-off-by: hridesh <hridesh699@gmail.com> > --- > rust/kernel/block/mq/request.rs | 7 +++---- > rust/kernel/rbtree.rs | 1 - > 2 files changed, 3 insertions(+), 5 deletions(-) > > diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs > index a0e22827f3f4..3ab2917c9d25 100644 > --- a/rust/kernel/block/mq/request.rs > +++ b/rust/kernel/block/mq/request.rs > @@ -22,15 +22,14 @@ > /// > /// There are four states for a request that the Rust bindings care about: > /// > -/// A) Request is owned by block layer (refcount 0) > +/// A) Request is owned by block layer (refcount 0)\ Instead of adding these backslashes, I personally would prefer if we make this a normal markdown list using `1.`, `2.` etc. Of course only if Andreas is OK with that though. --- Cheers, Benno > /// B) Request is owned by driver but with zero `ARef`s in existence > -/// (refcount 1) > +/// (refcount 1)\ > /// C) Request is owned by driver with exactly one `ARef` in existence > -/// (refcount 2) > +/// (refcount 2)\ > /// D) Request is owned by driver with more than one `ARef` in existence > /// (refcount > 2) > /// > -/// > /// We need to track A and B to ensure we fail tag to request conversions for > /// requests that are not owned by the driver. > /// > diff --git a/rust/kernel/rbtree.rs b/rust/kernel/rbtree.rs > index 25eb36fd1cdc..006f6e03aba5 100644 > --- a/rust/kernel/rbtree.rs > +++ b/rust/kernel/rbtree.rs > @@ -1031,7 +1031,6 @@ fn next(&mut self) -> Option<Self::Item> { > > /// A memory reservation for a red-black tree node. > /// > -/// > /// It contains the memory needed to hold a node that can be inserted into a red-black tree. One > /// can be obtained by directly allocating it ([`RBTreeNodeReservation::new`]). > pub struct RBTreeNodeReservation<K, V> { > -- > 2.46.0 >
On Mon, Sep 9, 2024 at 9:22 PM Benno Lossin <benno.lossin@proton.me> wrote: > > I don't know if the commit title should start with `docs`, maybe we want > to do `rust: docs` when changing rustdocs? (This is a question to the > other Rust reviewers) > > I think the title doesn't need to mention the exact cleanup, just > something along the lines "clean up docs" should suffice. Yeah, "docs" would be better for Documentation/ bits. I think something like this would be ideal: rust: kernel: clean up empty `\\\` lines > Instead of adding these backslashes, I personally would prefer if we Yeah, we have another patch on the list for that, so I would remove it from this one. Thanks! Cheers, Miguel
On Mon, Sep 9, 2024 at 6:19 PM hridesh <hridesh699@gmail.com> wrote: > > Signed-off-by: hridesh <hridesh699@gmail.com> Is hridesh a "known identity"? If not, please use your full legal name (please see https://docs.kernel.org/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin). Thanks! Cheers, Miguel
diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs index a0e22827f3f4..3ab2917c9d25 100644 --- a/rust/kernel/block/mq/request.rs +++ b/rust/kernel/block/mq/request.rs @@ -22,15 +22,14 @@ /// /// There are four states for a request that the Rust bindings care about: /// -/// A) Request is owned by block layer (refcount 0) +/// A) Request is owned by block layer (refcount 0)\ /// B) Request is owned by driver but with zero `ARef`s in existence -/// (refcount 1) +/// (refcount 1)\ /// C) Request is owned by driver with exactly one `ARef` in existence -/// (refcount 2) +/// (refcount 2)\ /// D) Request is owned by driver with more than one `ARef` in existence /// (refcount > 2) /// -/// /// We need to track A and B to ensure we fail tag to request conversions for /// requests that are not owned by the driver. /// diff --git a/rust/kernel/rbtree.rs b/rust/kernel/rbtree.rs index 25eb36fd1cdc..006f6e03aba5 100644 --- a/rust/kernel/rbtree.rs +++ b/rust/kernel/rbtree.rs @@ -1031,7 +1031,6 @@ fn next(&mut self) -> Option<Self::Item> { /// A memory reservation for a red-black tree node. /// -/// /// It contains the memory needed to hold a node that can be inserted into a red-black tree. One /// can be obtained by directly allocating it ([`RBTreeNodeReservation::new`]). pub struct RBTreeNodeReservation<K, V> {
Remove unnecessary empty `\\\` lines in the rust docs. Also add linebreaks in kernel::block::mq::Request to fix formatting Suggested-by: Miguel Ojeda <ojeda@kernel.org> Link: https://github.com/Rust-for-Linux/linux/issues/1109 Signed-off-by: hridesh <hridesh699@gmail.com> --- rust/kernel/block/mq/request.rs | 7 +++---- rust/kernel/rbtree.rs | 1 - 2 files changed, 3 insertions(+), 5 deletions(-)