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

Mark Lee (malept) 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.

=== 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?

=== 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)"?

> (wonder why there's no diff here)...

It looks like it's because the parent branch doesn't take into account my changing of the owner. I couldn't even `bzr branch` until I temporarily changed the owner of the main branch back to myself. Really stupid.

review: Needs Fixing (style, build system)

« Back to merge proposal