Rust for Linux List
 help / color / mirror / Atom feed
From: Gary Guo <gary@garyguo.net>
To: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Cc: "Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Wedson Almeida Filho" <wedsonaf@gmail.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] rust: make compiler-builtin stubs non-global
Date: Mon, 5 Dec 2022 21:40:54 +0000	[thread overview]
Message-ID: <20221205214054.6b9e6a7e@GaryWorkstation> (raw)
In-Reply-To: <CANiq72mXQed42DARuOjEipUffNxx97LRL+byZ0G2kjuRup6ZYQ@mail.gmail.com>

On Mon, 5 Dec 2022 21:26:27 +0100
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:

> 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.

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


  reply	other threads:[~2022-12-05 21:41 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-04 23:16 [PATCH v2] rust: make compiler-builtin stubs non-global Gary Guo
2022-12-05 20:26 ` Miguel Ojeda
2022-12-05 21:40   ` Gary Guo [this message]
2022-12-05 21:38 ` Nick Desaulniers

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=20221205214054.6b9e6a7e@GaryWorkstation \
    --to=gary@garyguo.net \
    --cc=alex.gaynor@gmail.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=wedsonaf@gmail.com \
    /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