Thread (5 messages) 5 messages, 2 authors, 2023-10-16

Re: [PATCH 1/1] cdrom: Add missing blank lines after declarations

From: Jens Axboe <axboe@kernel.dk>
Date: 2023-10-16 21:11:58

On 10/16/23 3:07 PM, Phillip Potter wrote:
On Mon, Oct 16, 2023 at 02:53:06PM -0600, Jens Axboe wrote:
quoted
On 10/16/23 2:47 PM, Phillip Potter wrote:
quoted
From: Edson Juliano Drosdeck <redacted>

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.
quoted
quoted
@@ -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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help