Re: [PATCH 22/348] Fix -Wsahdow warnings

[Doug Evans <dje@google.com>]

On Tue, Nov 22, 2011 at 9:29 PM, Andrey Smirnov
<andrew.smirnov@gmail.com> wrote:
> On Tue, Nov 22, 2011 at 10:27 PM, Mike Frysinger <vapier@gentoo.org> wrote:
>> On Tuesday 22 November 2011 09:04:07 Andrey Smirnov wrote:
>>> On Tue, Nov 22, 2011 at 8:35 PM, Marek Polacek <mpolacek@redhat.com> wrote:
>>> > On 11/22/2011 02:33 PM, Andrey Smirnov wrote:
>>> >>> Is the typo intentional?
>>> >>
>>> >> Sorry about that. No it isn't. Please ignore that particular patch I'll
>>> >> send corrected version soon(I'm in the middle of a git rebase
>>> >> --interactive, so I can't edit that particular commit just yet).
>>> >
>>> > That typo is in other patches too.
>>>
>>> OK, once again, sorry about that, I'll recheck all the patches related
>>> to bcache.c, for now please ignore those.
>>
>> please condense down your patches if you resend.  there's way too many little
>> tiny ones that really should be squashed into a single changeset.
>>
>
> Initially, there were 17 patches, which, upon suggestion from Tom
> Tromey, I split so that every patch contain only changes to one
> particular function or some other small unit of the source code. I
> tend to agree with Tom that my initial decision to make only 17
> patches made it rather hard to review each, because every one of them
> contained many small but disparate changes.
>
> Squashing or splitting commits is not really a problem and I can do
> this, but if you want me to do so, than please point out the patches
> I should squash together.
>
> Right now I have 258 patches to which I have yet to write a ChangeLog
> entry and 100 patches whose ChangeLog I have to change to, as you
> pointed out, conform to GNU policy. There are also patches that I
> screwed up with typos and patches that will eventually will have to be
> rewritten.
>
> Squashing all commits based on file level will still leave you with
> something like 150 patches, but some of them would be quite large and
> harder to review than they are now. Doing so on API level would
> require me to go through all the patches and relative source code,
> figure out to what API every change belong(which I'll probably do wrong
> because I do not yet have a very good grasp on GDB's internals) and
> split and squash all commits accordingly.
>
> So given the aforementioned amount of work, can't we ignore that the
> patch count is over 9000?
>
> Andrey Smirnov
>

For reference sake, I did "grep -e -Wall ChangeLog*" to see what's
been done in the past.  Based on that there is room for compromise I think.

Since these are just mechanical changes, and there are a lot of them,
I'd be happy with a compromise everyone is happy (or at least
not unhappy :-)) with.

I think keeping them at the file level is easiest for you (just
guessing though).
And I'd be happy with a changelog entry that simply said:

        * foo.c: -Wshadow lint.

or

        * foo.c (bar, baz): -Wshadow lint.
        (huey,dewey,louie): Ditto.

These are all just mechanical changes.

btw, I can imagine things regressing if we don't add -Wshadow
to configure.ac:build_warnings (say after the tree is clean).
We're agreed we want that right?  Otherwise ...

Also, given the quantity, I'd suggest holding off until after 7.4 is branched.
[just a suggestion though]