<div dir="ltr">
<p class="MsoNormal">Hi Vedant,</p><p class="MsoNormal"><br></p>
<p class="MsoNormal">Thanks for your explanation. </p>
<p class="MsoNormal"><br></p><p class="MsoNormal">> Based on your sample report, I think the second change
(repeating the summary<br>
statistics) is the only one that makes the report easier to read. This can be<br>
implemented with a one-line change.</p>
<p class="MsoNormal"><br></p><p class="MsoNormal">I would prefer to keep the current index report rather than moving/copying
the total table row to the top of the index table. So I have decided not to
make any changes.</p>
<p class="MsoNormal">I will abandon the D24687 Phabricator review.</p><p class="MsoNormal"><br></p>
<p class="MsoNormal">Thanks,</p>
</div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Sep 22, 2016 at 2:17 AM, Vedant Kumar <span dir="ltr"><<a href="mailto:vsk@apple.com" target="_blank">vsk@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">There are three changes here: display the longest common prefix of the<br>
filenames, repeat the summary statistics at the top of the page, and make the<br>
filename column only display one path component (leaving the rest to another<br>
column).<br>
<br>
Based on your sample report, I think the second change (repeating the summary<br>
statistics) is the only one that makes the report easier to read. This can be<br>
implemented with a one-line change.<br>
<br>
I don't think the other two changes contribute useful new information, or make<br>
reports easier to read. Knowing the precise location of the source directory<br>
doesn't matter when you're looking at the coverage for some file. This<br>
information is just noise if the source lives on a bot. Similarly, the new file<br>
directory column contains visually indistinct and redundant entries.<br>
<br>
I would rather not move forward with those two changes.<br>
<br>
thanks,<br>
vedant<br>
<div><div class="h5"><br>
> On Sep 21, 2016, at 10:48 AM, Ying Yi <<a href="mailto:maggieyi666@gmail.com">maggieyi666@gmail.com</a>> wrote:<br>
><br>
> Hi Vedant,<br>
><br>
><br>
><br>
> 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.<br>
><br>
><br>
><br>
> For example:<br>
><br>
> Project directory: I:\svn\PS4\toolchain-test\<wbr>tests\branches\PS4_4.500\<wbr>benchmarks<br>
> File directory: \Bullet-2.76\Demos\Benchmarks<br>
> The file BenchmarkDemo.cpp could find in the folder:<br>
> I:\svn\PS4\toolchain-test\<wbr>tests\branches\PS4_4.500\<wbr>benchmarks\Bullet-2.76\Demos\<wbr>Benchmarks .<br>
><br>
> Thanks,<br>
><br>
><br>
><br>
> On Tue, Sep 20, 2016 at 10:03 PM, Vedant Kumar <<a href="mailto:vsk@apple.com">vsk@apple.com</a>> wrote:<br>
> vsk added a comment.<br>
><br>
> > 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.<br>
><br>
><br>
> 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.<br>
><br>
><br>
> ================<br>
> Comment at: tools/llvm-cov/<wbr>SourceCoverageViewHTML.cpp:431<br>
> @@ -390,1 +430,3 @@<br>
> + OSRef << BeginCenteredDiv << BeginTable;<br>
> + emitColumnLabelsForIndex(<wbr>OSRef);<br>
> for (unsigned I = 0, E = FileReports.size(); I < E; ++I)<br>
> ----------------<br>
> 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?<br>
><br>
><br>
> <a href="https://reviews.llvm.org/D24687" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D24687</a><br>
><br>
><br>
><br>
><br>
><br>
><br>
> --<br>
> Ying Yi<br>
> SN Systems - Sony Interactive Entertainment<br>
</div></div>> <html_index.zip><br>
<br>
</blockquote></div><br><br clear="all"><br>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><font size="2"><span style="font-family:arial,helvetica,sans-serif"></span></font><font face="Calibri" size="2"><span style="font-size:11pt"><font size="2"><span style="font-size:10pt"><font color="#1F497D" face="Arial">Ying Yi<br>SN Systems - Sony Interactive Entertainment</font></span></font></span></font><br></div></div></div></div></div></div></div></div>
</div>