[prev in list] [next in list] [prev in thread] [next in thread]
List: gdb-patches
Subject: Re: [PATCH] Unify Solaris procfs and largefile handling
From: Rainer Orth <ro () CeBiTec ! Uni-Bielefeld ! DE>
Date: 2020-07-30 13:49:26
Message-ID: ydd36592jvd.fsf () CeBiTec ! Uni-Bielefeld ! DE
[Download RAW message or body]
Hi Simon,
>>> When I run `autoreconf -vf`, I get a lot of changes. Make sure to run
>>> it in any directory you touch that has a configure.ac and add the resulting
>>> changes.
>>
>> I didn't use autoreconf, but ran the appropriate
>> autoconf/autoheader/aclocal/automake dance manually. With one exception
>> (LARGEFILE_CPPFLAGS in gnulib Makefile.in) I'd gotten things right :-)
>>
>> I don't usually include generated files in patch submissions, though:
>> they are heavily frowned upon at least over in GCC because they make
>> review quite difficult, espcially in a case like this where the
>> largefile.m4 change spreads to lots of configure scripts, obscuring the
>> change proper.
>>
>> Does GDB handle things differently here?
>
> I don't think there's a hard rule. If it makes the patch too big for sending on
> the list, then it's fine for sure to not include them. If you don't want
> to include
> them, that's fine with my too, but in either case it's important to say
> that you've
> omitted them on purpose so we know it's not an oversight (otherwise I'll complain
> about them missing :)).
ok, will do.
> Maybe it can be inconvenient for people who read the diff directly to review... I
> personally use meld to review patches, so it's simple to just skip over generated
> files.
I'm using Emacs' Diff mode for that, which does nice highlighting.
However, having to skip over large parts of generated files is
inconvenient to me.
> The patch LGTM, thanks.
Pushed now. Thanks for the review.
Rainer
--
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic