diff mbox series

[COMMITTED] Makefile.am: Avoid code duplication

Message ID 20240509232428.608482-1-amerey@redhat.com
State Committed
Headers
Series [COMMITTED] Makefile.am: Avoid code duplication |

Commit Message

Aaron Merey May 9, 2024, 11:24 p.m. UTC 
  On Thu, May 9, 2024 at 6:59 PM Dmitry V. Levin <ldv@altlinux.org> wrote:
>
> On Thu, May 09, 2024 at 06:08:05PM -0400, Frank Ch. Eigler wrote:
> > Hi -
> >
> > On Fri, May 10, 2024 at 12:53:39AM +0300, Dmitry V. Levin wrote:
> > > > Pushed as commit ca8ad4648197
> > >
> > > What's the purpose of sending proposed patches to the mailing list
> > > if reviews are silently ignored?
> >
> > Please be collegial and don't exaggerate.
>
> The fact is that the review was silently ignored, which is, from my point
> of view, an extraordinary event, and I hereby raise the question why it
> was ignored, and request that the issue pointed out in the review to be
> properly addressed.

Apologies Dmitry, your review was mistakenly caught in my mail client's
spam filter and I did not see it until now.

I agree that we can avoid some code duplication here. I'm going to push
the following patch as obvious:

---
 Makefile.am | 33 ++++++---------------------------
 1 file changed, 6 insertions(+), 27 deletions(-)

Comments

Dmitry V. Levin May 9, 2024, 11:36 p.m. UTC | #1 
On Thu, May 09, 2024 at 07:24:26PM -0400, Aaron Merey wrote:
> On Thu, May 9, 2024 at 6:59 PM Dmitry V. Levin <ldv@altlinux.org> wrote:
> > On Thu, May 09, 2024 at 06:08:05PM -0400, Frank Ch. Eigler wrote:
> > > On Fri, May 10, 2024 at 12:53:39AM +0300, Dmitry V. Levin wrote:
> > > > > Pushed as commit ca8ad4648197
> > > >
> > > > What's the purpose of sending proposed patches to the mailing list
> > > > if reviews are silently ignored?
> > >
> > > Please be collegial and don't exaggerate.
> >
> > The fact is that the review was silently ignored, which is, from my point
> > of view, an extraordinary event, and I hereby raise the question why it
> > was ignored, and request that the issue pointed out in the review to be
> > properly addressed.
> 
> Apologies Dmitry, your review was mistakenly caught in my mail client's
> spam filter and I did not see it until now.
> 
> I agree that we can avoid some code duplication here. I'm going to push
> the following patch as obvious:
> 
> ---
>  Makefile.am | 33 ++++++---------------------------
>  1 file changed, 6 insertions(+), 27 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index a1f0b0c3..69af63ac 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -78,24 +78,16 @@ coverage: $(COVERAGE_OUTPUT_INDEX_HTML)
>  	@echo 'file://$(abs_builddir)/$(COVERAGE_OUTPUT_INDEX_HTML)'
>  
>  if LCOV_OLD
> -$(COVERAGE_OUTPUT_INDEX_HTML): $(COVERAGE_OUTPUT_FILE)
> -	LC_ALL=C $(GENHTML) \
> -		--legend \
> -		--show-details \
> -		--rc=genhtml_branch_coverage=1 \
> -		--title='$(COVERAGE_TITLE)' \
> -		--prefix='$(abspath $(abs_srcdir))' \
> -		--prefix='$(realpath $(abs_srcdir))' \
> -		--prefix='$(abspath $(abs_builddir)/..)' \
> -		--prefix='$(realpath $(abs_builddir)/..)' \
> -		--output-directory='$(COVERAGE_OUTPUT_DIRECTORY)' \
> -		$<
> +ignore_errors =
>  else
> +ignore_errors = --ignore-errors empty,negative
> +endif
> +
>  $(COVERAGE_OUTPUT_INDEX_HTML): $(COVERAGE_OUTPUT_FILE)
>  	LC_ALL=C $(GENHTML) \
>  		--legend \
>  		--show-details \
> -		--ignore-errors empty,negative \
> +		$(ignore_errors) \
>  		--rc=genhtml_branch_coverage=1 \
>  		--title='$(COVERAGE_TITLE)' \
>  		--prefix='$(abspath $(abs_srcdir))' \
> @@ -104,30 +96,17 @@ $(COVERAGE_OUTPUT_INDEX_HTML): $(COVERAGE_OUTPUT_FILE)
>  		--prefix='$(realpath $(abs_builddir)/..)' \
>  		--output-directory='$(COVERAGE_OUTPUT_DIRECTORY)' \
>  		$<
> -endif
>  
> -if LCOV_OLD
>  $(COVERAGE_OUTPUT_FILE):
>  	$(LCOV) \
>  		--capture \
>  		--no-external \
>  		--no-checksum \
> +		$(ignore_errors) \
>  		--rc=lcov_branch_coverage=1 \
>  		--gcov-tool='$(GCOV)' \
>  		--output-file='$@' \
>  		$(LCOV_DIRS_ARGS)
> -else
> -$(COVERAGE_OUTPUT_FILE):
> -	$(LCOV) \
> -		--capture \
> -		--no-external \
> -		--no-checksum \
> -		--ignore-errors empty,negative \
> -		--rc=lcov_branch_coverage=1 \
> -		--gcov-tool='$(GCOV)' \
> -		--output-file='$@' \
> -		$(LCOV_DIRS_ARGS)
> -endif
>  endif
>  
>  # Tell version 3.79 and up of GNU make to not build goals in this

That's much better, thanks!
diff mbox series

Patch

diff --git a/Makefile.am b/Makefile.am
index a1f0b0c3..69af63ac 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -78,24 +78,16 @@  coverage: $(COVERAGE_OUTPUT_INDEX_HTML)
 	@echo 'file://$(abs_builddir)/$(COVERAGE_OUTPUT_INDEX_HTML)'
 
 if LCOV_OLD
-$(COVERAGE_OUTPUT_INDEX_HTML): $(COVERAGE_OUTPUT_FILE)
-	LC_ALL=C $(GENHTML) \
-		--legend \
-		--show-details \
-		--rc=genhtml_branch_coverage=1 \
-		--title='$(COVERAGE_TITLE)' \
-		--prefix='$(abspath $(abs_srcdir))' \
-		--prefix='$(realpath $(abs_srcdir))' \
-		--prefix='$(abspath $(abs_builddir)/..)' \
-		--prefix='$(realpath $(abs_builddir)/..)' \
-		--output-directory='$(COVERAGE_OUTPUT_DIRECTORY)' \
-		$<
+ignore_errors =
 else
+ignore_errors = --ignore-errors empty,negative
+endif
+
 $(COVERAGE_OUTPUT_INDEX_HTML): $(COVERAGE_OUTPUT_FILE)
 	LC_ALL=C $(GENHTML) \
 		--legend \
 		--show-details \
-		--ignore-errors empty,negative \
+		$(ignore_errors) \
 		--rc=genhtml_branch_coverage=1 \
 		--title='$(COVERAGE_TITLE)' \
 		--prefix='$(abspath $(abs_srcdir))' \
@@ -104,30 +96,17 @@  $(COVERAGE_OUTPUT_INDEX_HTML): $(COVERAGE_OUTPUT_FILE)
 		--prefix='$(realpath $(abs_builddir)/..)' \
 		--output-directory='$(COVERAGE_OUTPUT_DIRECTORY)' \
 		$<
-endif
 
-if LCOV_OLD
 $(COVERAGE_OUTPUT_FILE):
 	$(LCOV) \
 		--capture \
 		--no-external \
 		--no-checksum \
+		$(ignore_errors) \
 		--rc=lcov_branch_coverage=1 \
 		--gcov-tool='$(GCOV)' \
 		--output-file='$@' \
 		$(LCOV_DIRS_ARGS)
-else
-$(COVERAGE_OUTPUT_FILE):
-	$(LCOV) \
-		--capture \
-		--no-external \
-		--no-checksum \
-		--ignore-errors empty,negative \
-		--rc=lcov_branch_coverage=1 \
-		--gcov-tool='$(GCOV)' \
-		--output-file='$@' \
-		$(LCOV_DIRS_ARGS)
-endif
 endif
 
 # Tell version 3.79 and up of GNU make to not build goals in this