From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21138 invoked by alias); 23 Nov 2011 17:56:38 -0000 Received: (qmail 21130 invoked by uid 22791); 23 Nov 2011 17:56:37 -0000 X-SWARE-Spam-Status: No, hits=-3.9 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,RCVD_IN_DNSWL_LOW,RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail-qw0-f41.google.com (HELO mail-qw0-f41.google.com) (209.85.216.41) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 23 Nov 2011 17:56:20 +0000 Received: by qap15 with SMTP id 15so745504qap.0 for ; Wed, 23 Nov 2011 09:56:19 -0800 (PST) Received: by 10.224.215.73 with SMTP id hd9mr11253642qab.94.1322070979591; Wed, 23 Nov 2011 09:56:19 -0800 (PST) MIME-Version: 1.0 Received: by 10.224.215.73 with SMTP id hd9mr11253587qab.94.1322070978616; Wed, 23 Nov 2011 09:56:18 -0800 (PST) Received: by 10.224.6.76 with HTTP; Wed, 23 Nov 2011 09:56:18 -0800 (PST) In-Reply-To: References: <878vn88fw3.fsf@gmail.com> <4ECBA525.1010801@redhat.com> <201111221027.52484.vapier@gentoo.org> Date: Wed, 23 Nov 2011 17:56:00 -0000 Message-ID: Subject: Re: [PATCH 22/348] Fix -Wsahdow warnings From: Doug Evans To: Andrey Smirnov Cc: Mike Frysinger , gdb-patches Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-System-Of-Record: true X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-11/txt/msg00630.txt.bz2 On Tue, Nov 22, 2011 at 9:29 PM, Andrey Smirnov wrote: > On Tue, Nov 22, 2011 at 10:27 PM, Mike Frysinger wrot= e: >> On Tuesday 22 November 2011 09:04:07 Andrey Smirnov wrote: >>> On Tue, Nov 22, 2011 at 8:35 PM, Marek Polacek wr= ote: >>> > 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. =A0there'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 branch= ed. [just a suggestion though]