linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jiri Slaby <jslaby@suse.cz>
To: Joe Perches <joe@perches.com>
Cc: Jiri Slaby <jirislaby@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Hansjoerg Lipp <hjlipp@web.de>, Tilman Schmidt <tilman@imap.cc>,
	gigaset307x-common@lists.sourceforge.net
Subject: Re: "whitespace coding style cleanup" broke coding style
Date: Mon, 27 Feb 2012 19:58:20 +0100	[thread overview]
Message-ID: <4F4BD24C.6000307@suse.cz> (raw)
In-Reply-To: <1330363627.9942.16.camel@joe2Laptop>

On 02/27/2012 06:27 PM, Joe Perches wrote:
> On Mon, 2012-02-27 at 18:12 +0100, Jiri Slaby wrote:
>> The patch should not
>> touch the code at all. It is obviously totally broken. In a separate
>> patch you might do s@,@;@ instead.
>
> The code in either form is neither broken nor
> incorrect.  It's just "out of style".
> Emacs did made it consistent.

I understand that it is correct according to the style. It is not 
correct in that how humans read it.

>> Hmm, but did not we conclude some time ago that we will not touch code
>> just to perform a whitespace cleanup?
>
> It's a prelude to other patches so actually
> that's done quite a lot.   git blame -w
> doesn't even show my name on any of the
> code in this patch.

Yeah, but git merge or rebase do not cope with this. That is the problem 
I am writing about.

>>> It was a first pass and an overall improvement.
>>
>> I hope no other passes are going to happen there or anywhere in TTY
>> drivers. I really do not want to solve zillion collisions in my ~100
>> local patches due to whitespace changes, sorry.
>
> Perhaps you should submit your ~100 patches sooner
> rather than later.  That's a lot of changes that
> could have any number of collisions.

No, sorry, I do not send patch series which are not reviewed by me with 
at least 2 weeks distance. Anyway the point is elsewhere.

To emphasize: I do not mind random changes on random files. That is easy 
to fix. I mind whitespace cleanups over _whole_ subtrees.

>> Yes, but it does not pass our brain, does it? One should throw
>> "checkpatch --file" or alike away, finally.
>
> Perhaps you might notice I did not use checkpatch
> as a guide nor as a criteria for submission.

Ok, but you still fit in "or alike" above.

> It was ~5MB patch btw.  Compilation was done to
> verify lack of object delta only.

I see. Sorry to say that, but 5MB of whitespace cleanup is purely 
insane. Do not tell me that you are going to fix/change something in all 
the files. And even if that is the case, fixes go first so that people 
are able to backport to stable if they fit there.

Please, search archives for similar discussions. I do not want to repeat 
that.

> Thanks for noticing the oddly formatted code.
> I'll send a patch to fix it.

It looks good. ACK. [Although I am sad I will have to solve that 
conflict once more :(.]

thanks,
-- 
js
suse labs

  parent reply	other threads:[~2012-02-27 18:58 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-27  8:59 "whitespace coding style cleanup" broke coding style Jiri Slaby
2012-02-27 11:19 ` Tilman Schmidt
2012-02-27 11:34   ` Jiri Slaby
2012-02-27 16:52 ` Joe Perches
2012-02-27 17:12   ` Jiri Slaby
2012-02-27 17:27     ` Joe Perches
2012-02-27 17:39       ` [PATCH net-next] gigaset: Use semicolons to terminate statements Joe Perches
2012-02-27 19:12         ` David Miller
2012-02-27 18:58       ` Jiri Slaby [this message]
2012-02-28 17:58   ` "whitespace coding style cleanup" broke coding style Tilman Schmidt
2012-02-28 22:05     ` Joe Perches
2012-02-28 23:08       ` Tilman Schmidt
2012-02-28 23:28         ` Joe Perches
2012-02-28 23:42           ` David Miller
2012-02-29  0:41             ` Alan Cox
2012-02-29  8:27           ` Armin Schindler
2012-02-29 19:32             ` Joe Perches
2012-02-29 11:05           ` Tilman Schmidt

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=4F4BD24C.6000307@suse.cz \
    --to=jslaby@suse.cz \
    --cc=gigaset307x-common@lists.sourceforge.net \
    --cc=hjlipp@web.de \
    --cc=jirislaby@gmail.com \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tilman@imap.cc \
    /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).