On 10/16/23 3:07 PM, Phillip Potter wrote:
> On Mon, Oct 16, 2023 at 02:53:06PM -0600, Jens Axboe wrote:
>> On 10/16/23 2:47 PM, Phillip Potter wrote:
>>> From: Edson Juliano Drosdeck <edson.drosdeck@xxxxxxxxx>
>>>
>>> Add missing blank lines after declarations to fix warning found by
>>> checkpatch.pl script.
>>
>> Let's please not do this. It's fine to run checkpatch on new patches to
>> ensure that you don't make mistakes, but this is just useless churn.
>> Even worse:
>>
> Hi Jens,
>
> So to be clear, I should not accept patches that do cleanup like this
> in future unless there are other substantive changes? I also build
> tested the patch as per normal.
Right. May be fine if actual fixes are being made, then do it as a prep
patch or something. That sometimes happens, you fix something and notice
that some styling is off and then fix that too.
>>> @@ -1202,6 +1204,7 @@ static int check_for_audio_disc(struct cdrom_device_info *cdi,
>>> {
>>> int ret;
>>> tracktype tracks;
>>> +
>>> cd_dbg(CD_OPEN, "entering check_for_audio_disc\n");
>>> if (!(cdi->options & CDO_CHECK_TYPE))
>>> return 0;
>>
>> This int ret is using spaces and not a tab, why even make a newline
>> change and not sort that out too?
>>
>
> Yes, good point. Given the patch only consisted of new lines though, I
> didn't think it a bad one. If this is the policy though, I will be
> stricter in future of course.
It's just pointless churn, which is the objection. Now granted for cdrom
the risk of conflict isn't that large, as it doesn't really see any
changes. But in general this is the kind of stuff that prevents stable
backports from just applying automatically. I view it similarly to
spelling fixes, and that kind of stuff. Is the style wrong? It is. But
it's not worth fixing seperately.
--
Jens Axboe
