Merge lp://staging/~vrruiz/gnome-control-center-signon/coverage-fixes into lp://staging/gnome-control-center-signon

Proposed by Víctor R. Ruiz
Status: Needs review
Proposed branch: lp://staging/~vrruiz/gnome-control-center-signon/coverage-fixes
Merge into: lp://staging/gnome-control-center-signon
Diff against target: 121 lines (+41/-27)
3 files modified
Makefile.am (+7/-20)
Makefile.am.coverage (+26/-0)
configure.ac (+8/-7)
To merge this branch: bzr merge lp://staging/~vrruiz/gnome-control-center-signon/coverage-fixes
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Needs Fixing
David King (community) Needs Fixing
Review via email: mp+137972@code.staging.launchpad.net

Description of the change

Move from --enable-lcov to --enable-gcov and make coverage-xml

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
David King (amigadave) wrote :

I do not see the Makefile.am.coverage file being added, and when I run autogen.sh from a clean checkout I get:

automake-1.11: cannot open < Makefile.am.coverage: No such file or directory

Did you test this? Additionally, you have provided no justification for this change in the description. Is it to match other webapps or Product Strategy projects, and if so, is the coverage Makefile.am snippet in a common repository anywhere?

review: Needs Fixing
126. By Víctor R. Ruiz

Add Makefile.am.coverage

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Víctor R. Ruiz (vrruiz) wrote :

Yes, I tested this, but I forgot to add the file. Already done. Makefile.am.coverage is the file we are using in PS QA with standard rules, available at lp:coverage-tutorial

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
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
127. By Víctor R. Ruiz

Fix make rule clean-gcda

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
128. By Víctor R. Ruiz

Silent support for Makefile.am.coverage

129. By Víctor R. Ruiz

Add gcovr check to configure.ac

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)

Unmerged revisions

129. By Víctor R. Ruiz

Add gcovr check to configure.ac

128. By Víctor R. Ruiz

Silent support for Makefile.am.coverage

127. By Víctor R. Ruiz

Fix make rule clean-gcda

126. By Víctor R. Ruiz

Add Makefile.am.coverage

125. By Víctor R. Ruiz

Standardized coverage make rules

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches