[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [PATCHv2] Kbuild: kconfig: Verbose version of --listnewconfig



Hi,

On Sat, Dec 4, 2010 at 1:30 PM, Ben Hutchings <ben@decadent.org.uk> wrote:
> On Sat, 2010-12-04 at 13:11 -0500, Arnaud Lacombe wrote:
> [...]
>> > +static void report_conf(struct menu *menu, bool verbose)
>> > +{
>> > +       struct symbol *sym;
>> > +       struct menu *child;
>> > +
>> > +       if (!menu_is_visible(menu))
>> > +               return;
>> > +
>> > +       if (verbose && menu == &rootmenu) {
>> > +               printf("\n#\n"
>> > +                      "# Changes:\n"
>> > +                      "#\n");
>> > +       }
>> > +
FWIW, some more nits about this header:
 - I'd rather either always or never see it, not depending on whether
or not KBUILD_VERBOSE is set to non-zero.
 - The "menu == &rootmenu" test is pretty useless, just move the
display (if any) outside of report_conf(), where there is no
ambiguity.
 - And why do you need a leading newline in front of the header (ie.
the "\n#\n") ? The only places where this construct is used in conf is
when there is a need to highlight the message, which should not be
needed here.

>> I would not expect to see any header if there is no new symbol(s).
>> However, that might complicate the code too much. Btw, I find
>> "Changes" to be misleading, is that header necessary ?
>
> We use this feature (or an earlier version of it) in automated kernel
> builds in Debian, so we expect the output to appear in build logs and
> the header makes it easier to pick out.
>
That's easily doable outside kconfig.

 - Arnaud


Reply to: