Re: [PATCH v2] rust: make compiler-builtin stubs non-global
From: Gary Guo
Date: Mon Dec 05 2022 - 16:41:09 EST
On Mon, 5 Dec 2022 21:26:27 +0100
Miguel Ojeda <miguel.ojeda.sandonis@xxxxxxxxx> wrote:
> On Mon, Dec 5, 2022 at 12:17 AM Gary Guo <gary@xxxxxxxxxxx> 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@xxxxxxxxxxx>
>
> 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.
Checked my bash history, and realised that there is a mistakenly added
`p` in my argument to `git format-patch` :)
>
> 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)?
People usually add new things to the bottom, and that's my rationale
for adding the note there.
>
> 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?
Will address, along with other suggestions, in v3.
Best,
Gary