Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221204231650.2604587-1-gary@garyguo.net>
In-Reply-To: <20221204231650.2604587-1-gary@garyguo.net>
From:   Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Date:   Mon, 5 Dec 2022 21:26:27 +0100
Message-ID: <CANiq72mXQed42DARuOjEipUffNxx97LRL+byZ0G2kjuRup6ZYQ@mail.gmail.com>
Subject: Re: [PATCH v2] rust: make compiler-builtin stubs non-global
To:     Gary Guo <gary@garyguo.net>
Cc:     Miguel Ojeda <ojeda@kernel.org>,
        Alex Gaynor <alex.gaynor@gmail.com>,
        Wedson Almeida Filho <wedsonaf@gmail.com>,
        Boqun Feng <boqun.feng@gmail.com>,
        =?UTF-8?Q?Bj=C3=B6rn_Roy_Baron?= <bjorn3_gh@protonmail.com>,
        rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Dec 5, 2022 at 12:17 AM Gary Guo <gary@garyguo.net> wrote:
>
> This patch was previous discussed on GitHub.

previous -> previously

> Link: https://github.com/Rust-for-Linux/linux/pull/779
>
> Signed-off-by: Gary Guo <gary@garyguo.net>

Hmm... Something happened to the patch (compared to v1): the triple
dashes, the diffstat, the base-commit etc. are not there. It can still
be applied, so no worries, but it feels a bit strange.

The newline between the tags should not be there. Instead, please put
it on top of `Link:` to keep tags separated (if you wanted to keep it
close to the sentence to refer to it, you can use "[1]" instead and
then "Link: https... [1]" in the tag).

> +// NOTE: if you are adding a new intrinsic is added here, you should also add it to
> +// `redirect-intrinsics` in `rust/Makefile`.

"is added" -> "". Also, what about putting the message at the top
instead (below the docs or perhaps after the macro but before the
calls to it)?

For the title of the patch, in the end I went with "rust: xxx: ..."
(where xxx is typically the name of the crate or the `kernel` crate
module), so perhaps "rust: compiler_builtins: make stubs non-global"
or similar?

Thanks a lot!

Cheers,
Miguel