[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAKUZ0z+mqGwyEt8oem7gLMXbNp6D3MPPMXEH5GCdA4_768d=5g@mail.gmail.com>
Date: Wed, 2 Apr 2025 08:27:00 -0400
From: Gabriel <gshahrouzi@...il.com>
To: Andy Shevchenko <andy.shevchenko@...il.com>
Cc: linux-media@...r.kernel.org, andy@...nel.org, hdegoede@...hat.com,
mchehab@...nel.org, sakari.ailus@...ux.intel.com, gregkh@...uxfoundation.org,
linux-kernel@...r.kernel.org, linux-staging@...ts.linux.dev
Subject: Re: [PATCH] staging: media: Fix indentation to use tabs instead of spaces
On Wed, Apr 2, 2025 at 3:12 AM Andy Shevchenko
<andy.shevchenko@...il.com> wrote:
>
> On Wed, Apr 2, 2025 at 5:40 AM <gshahrouzi@...il.com> wrote:
>
> Is it your first patch to the Linux kernel? See my comments below.
It's among the first patches I've submitted.
>
> > >From d6a08153171ac52b438b6ddc1da50ebdd3550951 Mon Sep 17 00:00:00 2001
> > From: Gabriel Shahrouzi <gshahrouzi@...il.com>
> > Date: Tue, 1 Apr 2025 22:04:25 -0400
> > Subject: [PATCH] staging: media: Fix indentation to use tabs instead of spaces
>
> First of all, your patch is mangled. You want to use proper tools,
> perhaps. One of which is `git format-patch ...` and another one is
> `git send-email ...`
I was using git format-patch but not git send-email which seems to
have been the issue. The meta-data from the patch was getting appended to
the top.
>
> > Replace spaces with tab to comply with kernel coding style.
>
> Change 'tab' to 'TAB'.
Got it.
>
> ...
>
> Change itself is okay, but is this the only one case in the entire
> driver (which is something like 100k LoCs long)? Even though, and
> while for the training purposes this is fine, you can also think about
> checking the common style of other functions, which may be shifted
> with TABs, but still having not enough spaces or so.
Good point. Regarding formatting, it probably makes the most sense to
address these issues comprehensively in a single cleanup pass (similar
to https://lore.kernel.org/linux-staging/cover.1743524096.git.karanja99erick@gmail.com/T/#t).
This particular instance caught my attention because I initially
thought the author might have accidentally used spaces instead of
tabs. The line in question used 2 tabs + 8 spaces, while subsequent
similarly-aligned lines used 3 tabs. However, after examining
different files in the driver, I noticed that while the formatting
appears inconsistent, it likely exists for specific reasons. It's
probably better to avoid changing a single detail without considering
the broader formatting approach, and to treat checkpatch.pl more as a
guide than the final authority.
>
>
> --
> With Best Regards,
> Andy Shevchenko
Powered by blists - more mailing lists