suggestion: minor change to maint.mk's 'coverage' targets to help develop coverage tests faster

2016-06-04 Thread Assaf Gordon
Hello,

I'd like to suggest the following minor change to the 'coverage' targets in 
'maint.mk'.

With it, when improving coverage of the test suite, a developer can bootstrap 
the coverage build with 'make coverage', then add individual test, then run 
"make re-coverage" and just re-generate the reports, without the need to 
rebuild and re-check the entire project.

Comments welcomed,
 - assaf

P.S.
An alternative is to move the 'lcov' command into the 'gen-coverage' target, 
but I wasn't sure if 'gen-coverage' is separate for some historical reason.




0001-maint.mk-add-intermediate-coverage-target.patch
Description: Binary data


Re: suggestion: minor change to maint.mk's 'coverage' targets to help develop coverage tests faster

2016-06-04 Thread Jim Meyering
On Sat, Jun 4, 2016 at 9:17 PM, Assaf Gordon  wrote:
> Hello,
>
> I'd like to suggest the following minor change to the 'coverage' targets in 
> 'maint.mk'.
>
> With it, when improving coverage of the test suite, a developer can bootstrap 
> the coverage build with 'make coverage', then add individual test, then run 
> "make re-coverage" and just re-generate the reports, without the need to 
> rebuild and re-check the entire project.

That patch looks fine. As before, please capitalize the first word of
each sentence in the commit log.

I was going to say you should add the new target name as a dependent
of .PHONY, so that "make -t" does not create the corresponding file,
but then saw that was not done for any of the coverage-related
targets.

It should be done for all of those targets, but not necessarily on this diff.

> P.S.
> An alternative is to move the 'lcov' command into the 'gen-coverage' target, 
> but I wasn't sure if 'gen-coverage' is separate for some historical reason.

I don't know about the rationale.



Re: suggestion: minor change to maint.mk's 'coverage' targets to help develop coverage tests faster

2016-06-04 Thread Jim Meyering
On Sat, Jun 4, 2016 at 11:26 PM, Jim Meyering  wrote:
> On Sat, Jun 4, 2016 at 9:17 PM, Assaf Gordon  wrote:
>> Hello,
>>
>> I'd like to suggest the following minor change to the 'coverage' targets in 
>> 'maint.mk'.
>>
>> With it, when improving coverage of the test suite, a developer can 
>> bootstrap the coverage build with 'make coverage', then add individual test, 
>> then run "make re-coverage" and just re-generate the reports, without the 
>> need to rebuild and re-check the entire project.
>
> That patch looks fine. As before, please capitalize the first word of
> each sentence in the commit log.
>
> I was going to say you should add the new target name as a dependent
> of .PHONY, so that "make -t" does not create the corresponding file,
> but then saw that was not done for any of the coverage-related
> targets.
>
> It should be done for all of those targets, but not necessarily on this diff.
>
>> P.S.
>> An alternative is to move the 'lcov' command into the 'gen-coverage' target, 
>> but I wasn't sure if 'gen-coverage' is separate for some historical reason.
>
> I don't know about the rationale.

One more thing: for this line in the commit log,

  (re-coverage): new target, re-captures coverage data and generates update

I think you want a "d" at the end:

  (re-coverage): New target, re-captures coverage data and generates updated
  coverage HTML reports.