Code review comment for lp://staging/~vrruiz/gnome-control-center-signon/coverage-fixes

Revision history for this message
David King (amigadave) wrote :

OK, but there are a few problems with this that should be fixed:

* it does not support the automake silent rule predefined variables (for example, AM_V_at and AM_V_GEN)
* it adds a dependency on gcovr and does not check for it in configure.ac
* it redefines the "clean-local" and ".PHONY" targets that are already defined in Makefile.am
* find does not need to use xargs, it can call rm directly
* "pwd" is used in the "generate-coverage-xml" rule, when one of the standard automake variables should be used (either "top_builddir" or "top_srcdir" I guess)
* it uses echo to print some messages to stdout, which ignores the silent rules setting. if those are desired, some custom verbosity rules should be added instead, much like the automake silent rule predefined variables

review: Needs Fixing

« Back to merge proposal