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

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



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: signature.asc
Description: This is a digitally signed message part


Reply to: