Merge lp:~allanlesage/indicator-messages/TDD into lp:indicator-messages/0.3

Proposed by Product Strategy Coverity Bug Uploader on 2012-04-16
Status: Merged
Approved by: Charles Kerr on 2012-04-18
Approved revision: 260
Merged at revision: 271
Proposed branch: lp:~allanlesage/indicator-messages/TDD
Merge into: lp:indicator-messages/0.3
Diff against target: 87 lines (+23/-15)
2 files modified
Makefile.am.coverage (+8/-4)
configure.ac (+15/-11)
To merge this branch: bzr merge lp:~allanlesage/indicator-messages/TDD
Reviewer Review Type Date Requested Status
Charles Kerr (community) 2012-04-16 Approve on 2012-04-18
Review via email: mp+102159@code.launchpad.net

Description of the change

Changes revised under Charles' direction; Charles I've one diff beyond yours, removing an obsolete configuration, would you verify that this is correct please?

To post a comment you must log in.
Charles Kerr (charlesk) wrote :

Allan, I don't usually worry about indentation but the indentation in this patch is actively confusing... it looks like a "fi" is missing but isn't. Could you line up the if..elif..fi statements with each other?

review: Needs Fixing
260. By Allan LeSage on 2012-04-17

Clarified tabination.

Allan LeSage (allanlesage) wrote :

Justly retabinated, thanks Charles for your review.

Charles Kerr (charlesk) wrote :

Thanks Allan :)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Makefile.am.coverage'
2--- Makefile.am.coverage 2012-03-19 18:02:04 +0000
3+++ Makefile.am.coverage 2012-04-17 15:28:18 +0000
4@@ -1,14 +1,18 @@
5
6 # Coverage targets
7
8-.PHONY: clean-gcda \
9+.PHONY: clean-gcno clean-gcda \
10 coverage-html generate-coverage-html clean-coverage-html \
11 coverage-gcovr generate-coverage-gcovr clean-coverage-gcovr
12
13-clean-local: clean-coverage-html clean-coverage-gcovr
14+clean-local: clean-gcno clean-coverage-html clean-coverage-gcovr
15
16 if HAVE_GCOV
17
18+clean-gcno:
19+ @echo Removing old coverage instrumentation
20+ -find -name '*.gcno' -print | xargs -r rm
21+
22 clean-gcda:
23 @echo Removing old coverage results
24 -find -name '*.gcda' -print | xargs -r rm
25@@ -34,10 +38,10 @@
26
27 generate-coverage-gcovr:
28 @echo Generating coverage GCOVR report
29- $(GCOVR) -x -r $(top_builddir) -o $(top_builddir)/coverage.gcovr
30+ $(GCOVR) -x -r $(top_builddir) -o $(top_builddir)/coverage.xml
31
32 clean-coverage-gcovr: clean-gcda
33- -rm -rf $(top_builddir)/coverage.gcovr
34+ -rm -rf $(top_builddir)/coverage.xml
35
36 endif # HAVE_GCOVR
37
38
39=== modified file 'configure.ac'
40--- configure.ac 2012-04-11 16:00:45 +0000
41+++ configure.ac 2012-04-17 15:28:18 +0000
42@@ -77,15 +77,6 @@
43 AC_SUBST(APPLET_LIBS)
44
45 ###########################
46-# Test Dependencies
47-###########################
48-
49-AC_ARG_ENABLE([tests],
50- AC_HELP_STRING([--disable-tests], [Disable test scripts and tools]),,
51- [enable_tests=auto])
52-AM_CONDITIONAL(BUILD_TESTS, test xyes = xyes)
53-
54-###########################
55 # Status Provider Deps
56 ###########################
57
58@@ -124,8 +115,20 @@
59 # Google Test framework
60 ###########################
61
62-m4_include([m4/gtest.m4])
63-CHECK_GTEST
64+AC_ARG_ENABLE([tests],
65+ [AS_HELP_STRING([--disable-tests], [Disable test scripts and tools (default=auto)])],
66+ [enable_tests=${enableval}],
67+ [enable_tests=auto])
68+if test "x$enable_tests" != "xno"; then
69+ m4_include([m4/gtest.m4])
70+ CHECK_GTEST
71+ if test "x$enable_tests" = "xauto"; then
72+ enable_tests=${have_gtest}
73+ elif test "x$enable_tests" = "xyes" && test "x$have_gtest" != "xyes"; then
74+ AC_MSG_ERROR([tests were requested but gtest is not installed.])
75+ fi
76+fi
77+AM_CONDITIONAL([BUILD_TESTS],[test "x$enable_tests" = "xyes"])
78
79 ###########################
80 # Check to see if we're local
81@@ -229,5 +232,6 @@
82
83 Prefix: $prefix
84 Indicator Dir: $INDICATORDIR
85+ gtest: $enable_tests
86 gcov: $use_gcov
87 ])

Subscribers

People subscribed via source and target branches