Code review comment for lp:~awn-core/libdesktop-agnostic/various-fixes

Michal Hruby (mhr3) wrote :

> > I'd like Mark to look at the addition of "--disable-gi" configure option,
> > I'm sure it could use some cleanup.
>
> Done, among other things:
>
> === libdesktop-agnostic/wscript:
> > + if bld.env['INTROSPECTION']: lib.gir = 'DesktopAgnostic-1.0'
>
> Two lines, please (PEP8 applies). Same for all of the other instances.

Done.

>
> === python/desktopagnostic.override:
> + if (ret != NULL)
> + g_object_unref (ret);
>
> Could someone please add curly braces to all of the statements which don't
> have them?

This is the style rest of the python code generation uses.

>
> === wscript:
>
> + opt.add_option('--disable-gi', action='store_true',
> + dest='no_gi', default=False)
>
> I generally don't like the idea of variable names with negatives in them.
> Could it change to be "dest='enable_gi', default=True)"?
>

I tried, but waf is too stupid to figure out that --disable-xyz is opposite of --enable-xyz. And no it doesn't support --enable-xyz=no/false.

« Back to merge proposal