On Sat, 2010-12-04 at 14:53 -0500, Arnaud Lacombe wrote:
> 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 other users of listnewconfig obviously didn't want that.
> - 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.
No it isn't, as the Kconfig code must be built as part of the
listnewconfig target. (Building just the code first will provoke
warnings about the invalid config.)
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