[PATCH] rust: sort includes in bindings_helper.h

[PATCH] rust: sort includes in bindings_helper.h

[Alice Ryhl]

Dash has ascii value 45 and underscore has ascii value 95, so to
correctly sort the includes, the underscore should be last.

Fixes: 3253aba3408a ("rust: block: introduce `kernel::block::mq` module")
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/bindings/bindings_helper.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index b940a5777330..ae82e9c941af 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -7,8 +7,8 @@
  */
 
 #include <kunit/test.h>
-#include <linux/blk_types.h>
 #include <linux/blk-mq.h>
+#include <linux/blk_types.h>
 #include <linux/blkdev.h>
 #include <linux/errname.h>
 #include <linux/ethtool.h>
-- 
2.46.0.76.ge559c4bf1a-goog

Re: [PATCH] rust: sort includes in bindings_helper.h

[Miguel Ojeda]

On Fri, Aug 9, 2024 at 8:42 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> Dash has ascii value 45 and underscore has ascii value 95, so to
> correctly sort the includes, the underscore should be last.
>
> Fixes: 3253aba3408a ("rust: block: introduce `kernel::block::mq` module")

Looks good to me (`LC_ALL=C`), thanks!

I can take it; otherwise:

    Acked-by: Miguel Ojeda <ojeda@kernel.org>

I am not sure if this should count as a bug/fix (there is an
recent/ongoing debate about the Fixes tag).

(This kind of issues can be also opened as "good first issues", by the
way, i.e. as a way to get contributors to set their email workflow.)

Cheers,
Miguel

Re: [PATCH] rust: sort includes in bindings_helper.h

[Alice Ryhl]

On Fri, Aug 9, 2024 at 2:58 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Fri, Aug 9, 2024 at 8:42 AM Alice Ryhl <aliceryhl@google.com> wrote:
> >
> > Dash has ascii value 45 and underscore has ascii value 95, so to
> > correctly sort the includes, the underscore should be last.
> >
> > Fixes: 3253aba3408a ("rust: block: introduce `kernel::block::mq` module")
>
> Looks good to me (`LC_ALL=C`), thanks!
>
> I can take it; otherwise:
>
>     Acked-by: Miguel Ojeda <ojeda@kernel.org>
>
> I am not sure if this should count as a bug/fix (there is an
> recent/ongoing debate about the Fixes tag).

I fix merge conflicts in this file almost daily, so I think there's a
case to be made for taking it as a fix. I should have clarified this
in my commit message. I sent a v2 with more info:
https://lore.kernel.org/r/20240809132835.274603-1-aliceryhl@google.com

> (This kind of issues can be also opened as "good first issues", by the
> way, i.e. as a way to get contributors to set their email workflow.)

I didn't think of that, but if I had I would probably still have
submitted it myself for the above reason.

Alice

Re: [PATCH] rust: sort includes in bindings_helper.h

[Danilo Krummrich]

On 8/9/24 8:42 AM, Alice Ryhl wrote:
> Dash has ascii value 45 and underscore has ascii value 95, so to
> correctly sort the includes, the underscore should be last.
> 
> Fixes: 3253aba3408a ("rust: block: introduce `kernel::block::mq` module")

I don't think this patch needs a "Fixes" tag, it's usually for bugs only.

But it still makes sense to mention the commit that introduced the include
in the commit message.

> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

Generally speaking, unless minor style issues cause compiler or linter warnings,
I think it's better to leave them alone in favor of not messing with git-blame.

In this case we're not hiding something relevant though, hence

Acked-by: Danilo Krummrich <dakr@kernel.org>

> ---
>   rust/bindings/bindings_helper.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index b940a5777330..ae82e9c941af 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -7,8 +7,8 @@
>    */
>   
>   #include <kunit/test.h>
> -#include <linux/blk_types.h>
>   #include <linux/blk-mq.h>
> +#include <linux/blk_types.h>
>   #include <linux/blkdev.h>
>   #include <linux/errname.h>
>   #include <linux/ethtool.h>

Re: [PATCH] rust: sort includes in bindings_helper.h

[Alice Ryhl]

On Fri, Aug 9, 2024 at 3:01 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On 8/9/24 8:42 AM, Alice Ryhl wrote:
> > Dash has ascii value 45 and underscore has ascii value 95, so to
> > correctly sort the includes, the underscore should be last.
> >
> > Fixes: 3253aba3408a ("rust: block: introduce `kernel::block::mq` module")
>
> I don't think this patch needs a "Fixes" tag, it's usually for bugs only.
>
> But it still makes sense to mention the commit that introduced the include
> in the commit message.

Ok. I can make this change.

Alice

Re: [PATCH] rust: sort includes in bindings_helper.h

[Jens Axboe]

On 8/9/24 12:42 AM, Alice Ryhl wrote:
> Dash has ascii value 45 and underscore has ascii value 95, so to
> correctly sort the includes, the underscore should be last.

This commit message lacks an explanation for why the change is
being done. Yes it states that it brings the headers in ascii
sort order, but WHY?

-- 
Jens Axboe

Re: [PATCH] rust: sort includes in bindings_helper.h

[Alice Ryhl]

On Fri, Aug 9, 2024 at 3:07 PM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 8/9/24 12:42 AM, Alice Ryhl wrote:
> > Dash has ascii value 45 and underscore has ascii value 95, so to
> > correctly sort the includes, the underscore should be last.
>
> This commit message lacks an explanation for why the change is
> being done. Yes it states that it brings the headers in ascii
> sort order, but WHY?

I can add the following to the commit message:

The headers in this file are sorted alphabetically, which makes it
easy to quickly resolve conflicts by selecting all of the headers and
invoking :'<,'>sort to sort them. To keep this technique to resolve
conflicts working, also apply sorting to symbols that are not letters.

Alice