[PATCH] D24687: [llvm-cov] Add a project coverage summary above the index page.

[Vedant Kumar via llvm-commits]

There are three changes here: display the longest common prefix of the
filenames, repeat the summary statistics at the top of the page, and make the
filename column only display one path component (leaving the rest to another
column).

Based on your sample report, I think the second change (repeating the summary
statistics) is the only one that makes the report easier to read. This can be
implemented with a one-line change.

I don't think the other two changes contribute useful new information, or make
reports easier to read. Knowing the precise location of the source directory
doesn't matter when you're looking at the coverage for some file. This
information is just noise if the source lives on a bot. Similarly, the new file
directory column contains visually indistinct and redundant entries.

I would rather not move forward with those two changes.

thanks,
vedant

> On Sep 21, 2016, at 10:48 AM, Ying Yi <maggieyi666 at gmail.com> wrote:
> 
> Hi Vedant,
> 
> 
> 
> I have attached the index.html page which includes the project directory in the summary, and separate columns for files and directories in the index table. Listing the project directory in summary allows user to construct the full file path.
> 
> 
> 
> For example:
> 
> Project directory: I:\svn\PS4\toolchain-test\tests\branches\PS4_4.500\benchmarks
> File directory: \Bullet-2.76\Demos\Benchmarks
>  The file BenchmarkDemo.cpp could find in the folder:
>  I:\svn\PS4\toolchain-test\tests\branches\PS4_4.500\benchmarks\Bullet-2.76\Demos\Benchmarks .
> 
> Thanks,
> 
> 
> 
> On Tue, Sep 20, 2016 at 10:03 PM, Vedant Kumar <vsk at apple.com> wrote:
> vsk added a comment.
> 
> > If the project is branched, the longest-common prefix of the path (I called the project directory) will show the branch name. The project directory will tell the user where the source code come from.
> 
> 
> It would be easier for me to understand the motivation behind this if you could upload an example report with your changes. I still don't understand what it is about the current scheme that makes it difficult for readers to figure out where some source file comes from.
> 
> 
> ================
> Comment at: tools/llvm-cov/SourceCoverageViewHTML.cpp:431
> @@ -390,1 +430,3 @@
> +  OSRef << BeginCenteredDiv << BeginTable;
> +  emitColumnLabelsForIndex(OSRef);
>    for (unsigned I = 0, E = FileReports.size(); I < E; ++I)
> ----------------
> This patch still seems larger than it needs to be. Can't we get the entire desired functional change by just inserting another `emitFileSummary(OSRef, "Totals", Totals, /*IsTotals=*/true)` here?
> 
> 
> https://reviews.llvm.org/D24687
> 
> 
> 
> 
> 
> 
> -- 
> Ying Yi
> SN Systems - Sony Interactive Entertainment
> <html_index.zip>