GNOME Bugzilla – Bug 747699
some ide-builder patches
Last modified: 2015-04-11 23:07:09 UTC
I started looking at this code and poked around a bit. Feel free to apply or discard the following patches.
Created attachment 301361 [details] [review] patch trivial typo
Created attachment 301362 [details] [review] patch 2 This fixes the todo in the workbench about it generic and pushing down autotools specific implementation details to the concrete builder. I am not sure this goes in the direction you have in mind since it adds a flags argument while your approach was to pass down a free form "config" object. Clearly the generic config object is much more flexible and an alternative would be to set the "force-rebuild" in the config object but in a section more generic than "autotools", however since "force rebuild" is something that will be shared among all kind of builders and even reflected in the UI, I think making it explicit in "flags" could work.
Created attachment 301363 [details] [review] patch 3 Remove boiler plate from ide-builder.c and make abstract. Just removing the empty set/get_prop, finalize etc that looks like generated from a snippet. Not sure if it is worth it, since I guess you have plenty of this stuff around, but seems unlikely that this base class need then any time soon. Maybe slightly more interesting, declare the type abstract since there is not even a default implementation of build_async
Review of attachment 301361 [details] [review]: Perfect example of why not to use a generic container for everything :)
Review of attachment 301362 [details] [review]: LGTM. ide-build.c was such a hack. If we are going to end up installing it with bin_PROGRAMS someday, it really should be cleaned up.
Review of attachment 301363 [details] [review]: LGTM
Created attachment 301377 [details] [review] patch 4
Review of attachment 301377 [details] [review]: fine, seen on irc with Christian.
Review of attachment 301377 [details] [review]: commited as b65749b
pushed the patches