diff mbox series

Makefile: add a prerequisite to the coverage-report target

Message ID 20220408105443.192217-1-gitter.spiros@gmail.com (mailing list archive)
State Superseded
Headers show
Series Makefile: add a prerequisite to the coverage-report target | expand

Commit Message

Elia Pinto April 8, 2022, 10:54 a.m. UTC
Directly invoking make coverage-report as a target results in an error because
its prerequisites are missing,

The patch adds the necessary prerequisite.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Junio C Hamano April 8, 2022, 8:10 p.m. UTC | #1
Elia Pinto <gitter.spiros@gmail.com> writes:

> @@ -3409,7 +3409,7 @@ coverage-prove: coverage-clean-results coverage-compile
>  		DEFAULT_TEST_TARGET=prove GIT_PROVE_OPTS="$(GIT_PROVE_OPTS) -j1" \
>  		-j1 test
>  
> -coverage-report:
> +coverage-report: coverage-test
>  	$(QUIET_GCOV)for dir in $(object_dirs); do \
>  		$(GCOV) $(GCOVFLAGS) --object-directory=$$dir $$dir*.c || exit; \
>  	done

I am not sure if this alone makes it a good change.

With the current set-up, you can run coverage-test or coverage-prove
once, view coverage-report, shift your attention to elsewhere to
address issues you saw in the report, and then decide to view the
report again to remind yourself what you saw and what motivated you
to work on your changes.  Most likely this sequence would be
followed by another run of coverage-test followed by coverage-report
to compare what you saw before you made these changes with the
covernage report after your changes.

If this were conditional, i.e. "ah, I see you haven't run any
coverage test yet, so let me run it for you before showing the
result" combined with "ok, I see you did run coverage test, so let
me just show the result without running tests anew", then it would
be a strict improvement from the status quo.

But with the patch as posted, wouldn't we unconditonally wipe the
earlier result out?  If so that may negatively affect established
workflow of some people, I am afraid.

Thanks.
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index e8aba291d7..d9e8dd331e 100644
--- a/Makefile
+++ b/Makefile
@@ -3409,7 +3409,7 @@  coverage-prove: coverage-clean-results coverage-compile
 		DEFAULT_TEST_TARGET=prove GIT_PROVE_OPTS="$(GIT_PROVE_OPTS) -j1" \
 		-j1 test
 
-coverage-report:
+coverage-report: coverage-test
 	$(QUIET_GCOV)for dir in $(object_dirs); do \
 		$(GCOV) $(GCOVFLAGS) --object-directory=$$dir $$dir*.c || exit; \
 	done