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

[Ben Hutchings <ben@decadent.org.uk>]

[-- Attachment #1: Type: text/plain, Size: 1499 bytes --]

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");
> > +       }
> > +
> 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.

> > +       sym = menu->sym;
> > +       if (sym && (sym->flags & SYMBOL_NEW) &&
> > +           sym_is_changable(sym) && sym->name && !sym_is_choice_value(sym)) {
> > +               if (verbose)
> > +                       conf_write_symbol(sym, sym->type, stdout, true);
> > +               else
> > +                       printf("CONFIG_%s\n", sym->name);
> Please don't hardcode the prefix string. This should be:
> 
>                        printf("%s%s\n", CONFIG_, sym->name);

OK, but I don't understand why it would change.

Ben.

-- 
Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]