suggestion: minor change to maint.mk's 'coverage' targets to help develop coverage tests faster
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
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
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.