Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:80233
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.216.174 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAF+90c8eNPGFZ+pf838QXJHc6j90xnOM-GWiFfn8P=hXbeKcVg@mail.gmail.com>
References: <54AB172E.6040901@gmail.com>
	<CAF+90c89ES=C17FSbEwF+9zoudVpR+WE_TG7x6QmVqC--GnTuA@mail.gmail.com>
	<alpine.DEB.2.11.1501061125460.4080@whisky.home.derickrethans.nl>
	<CAF+90c8eNPGFZ+pf838QXJHc6j90xnOM-GWiFfn8P=hXbeKcVg@mail.gmail.com>
Date: Tue, 6 Jan 2015 03:43:47 -0800
Message-ID: <CAEZPtU6GvkErd-YpAUeK4VbSFwW4ro0QbKJXnezKqJwGH-ZDXg@mail.gmail.com>
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: Derick Rethans <derick@php.net>, Stanislav Malyshev <smalyshev@gmail.com>, 
	PHP Internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] Cleaning up dangling whitespace
From: pierre.php@gmail.com (Pierre Joye)

On Tue, Jan 6, 2015 at 3:33 AM, Nikita Popov <nikita.ppv@gmail.com> wrote:
> On Tue, Jan 6, 2015 at 12:26 PM, Derick Rethans <derick@php.net> wrote:
>
>> On Tue, 6 Jan 2015, Nikita Popov wrote:
>>
>> > On Mon, Jan 5, 2015 at 11:58 PM, Stanislav Malyshev <smalyshev@gmail.com
>> >
>> > wrote:
>> >
>> > > The code in the PHP source repo has a lot of random dangling
>> > > whitespace at the end of lines, which looks sloppy and produces some
>> > > weird effects in diffs because some editors auto-clean it and some
>> > > do not. I'd like to clean it up (it's easy to do automatically) in
>> > > master, but this produces a big patch that may also create some
>> > > conflicts with existing pull reqs (easily resolved, but still). So,
>> > > I'd like to ask if there are any objections to such cleanup? If not,
>> > > then I'll do it, probably next weekend.
>>
>> Don't do it for just master, it makes automatic merging possibly a pain.
>> It should go to all "alive" branches, with properly merged up commits.
>>
>> > No objection to whitespace cleanup. iirc git even has options to
>> > ignore whitespace changes on merge/rebase.
>> >
>> > It would be nice to also add a pre-commit hook stripping trailing
>> > whitespace, to avoid this happening again.
>>
>> That would change a commit hash, or cause an extra commit. No thanks.
>>
>
> This is not how pre-commit hooks work.
>
> If we only want to check (as opposed to automatically fix), it would be
> enough to recommend enabling the default pre-commit hook in our git
> guidelines.

Even better than a server side hook, far less intrusive and rejecting
a commit when CS are broken before it even gets applied locally sounds
like the best solution so far.

For the reference, how to make it:
http://lostechies.com/jasonmeridth/2009/09/10/git-local-pre-commit-hook/

Now we only need a lint cmd call checking the PHP internals CS :)


-- 
Pierre

@pierrejoye | http://www.libgd.org