rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
To: Danilo Krummrich <dakr@redhat.com>
Cc: Alice Ryhl <aliceryhl@google.com>,
	a.hindborg@samsung.com, alex.gaynor@gmail.com,
	 benno.lossin@proton.me, bjorn3_gh@protonmail.com,
	boqun.feng@gmail.com,  gary@garyguo.net,
	linux-kernel@vger.kernel.org, ojeda@kernel.org,
	 rust-for-linux@vger.kernel.org, wedsonaf@gmail.com
Subject: Re: [PATCH v4] rust: str: add {make,to}_{upper,lower}case() to CString
Date: Tue, 20 Feb 2024 17:53:51 +0100	[thread overview]
Message-ID: <CANiq72m7-5CuKAKDk=G9sauri_sH0nR2TXi8ye5Eji-qaum3sw@mail.gmail.com> (raw)
In-Reply-To: <462aad75-4f03-4f8b-ad58-eef429ed2b34@redhat.com>

On Tue, Feb 20, 2024 at 4:53 PM Danilo Krummrich <dakr@redhat.com> wrote:
>
> The rational for a convention can't be that it is a convention. Instead
> it should be a convention for an objective reason.

The rationale __for the lint__ is that it is a very established
convention in Rust code.

That is what Clippy is telling you.

You may not agree with the convention (and thus the lint). That is
fine, but it is still a fact that it is the convention, and that is
why I said whoever wrote that Clippy description probably felt that
wording is good enough.

> I'm not saying that we should enforce it otherwise, I just think that we
> should have objective reasons for restrictions.

Again, you seem to be saying we do not have objective reasons.

> However, I also don't see why we should disallow it as long as there is
> no objective reason to do so.

There are objective reasons to do so and we already explained them to you above:

    https://lore.kernel.org/rust-for-linux/CAH5fLggXiXGA_UWCxqqLhMpXe1kepDv2vMG+1jLGaDSzdRHvSw@mail.gmail.com/
    https://lore.kernel.org/rust-for-linux/CANiq72nVkV3+1rt4Mi+Own6KGAzmvR2jf8fFsp9NBu_gy_ob5g@mail.gmail.com/

You disagree? Fine, if you can come up with an argument that convinces
us or a good majority of the kernel that uses Rust, then I will
happily apply the patch.

Personally, I don't particularly care (based on style arguments alone)
whether to write `return` or not. But I do care about not wasting time
on litigating particular choices each time somebody does not like one.

> That's actually what the language did already with early-return vs return at
> the end of the function.
>
> I admit that consistent inconsistency is also kinda consistent though. :-)
>
> The language has various characteristics, maybe that's why it allows both?

Languages may allow a lot of things, but that does not mean we write
the code like IOCCC just because we can.

Nor it does mean that a project should allow every way to write things
either, _especially_ when it is a very widespread convention.

> See above.

I don't see it, sorry. You said you want to be "explicit", but one can
very easily argue you are _not_ being explicit by omitting the
`return` in some cases.

> That's great, although I really don't understand why you think this one is, but
> the other one isn't. What's the difference?

We already told you in this thread several times -- the `return` one
is idiomatic and so on. See the links above.

> I agree, but I also think it should be the other way around. We should have good
> examples and an objective rationale for things we restrict.

Why? To me, we should have good examples and an objective rationale
for things where we actually want to deviate from what everybody else
is doing.

Cheers,
Miguel

  reply	other threads:[~2024-02-20 16:54 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-19 16:39 [PATCH v4] rust: str: add {make,to}_{upper,lower}case() to CString Danilo Krummrich
2024-02-20  9:35 ` Alice Ryhl
2024-02-20 12:02   ` Danilo Krummrich
2024-02-20 13:17     ` Alice Ryhl
2024-02-20 14:53       ` Danilo Krummrich
2024-02-20 15:45         ` Miguel Ojeda
2024-02-20 19:55           ` Danilo Krummrich
2024-02-20 15:04     ` Miguel Ojeda
2024-02-20 15:53       ` Danilo Krummrich
2024-02-20 16:53         ` Miguel Ojeda [this message]
2024-02-20 19:55           ` Danilo Krummrich

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='CANiq72m7-5CuKAKDk=G9sauri_sH0nR2TXi8ye5Eji-qaum3sw@mail.gmail.com' \
    --to=miguel.ojeda.sandonis@gmail.com \
    --cc=a.hindborg@samsung.com \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=benno.lossin@proton.me \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dakr@redhat.com \
    --cc=gary@garyguo.net \
    --cc=linux-kernel@vger.kernel.org \
    --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;
as well as URLs for NNTP newsgroup(s).