-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add HtmlTransform-v3.xslt and update HtmlSummary-v3.xslt to make html consistent #10
Conversation
…2 consistent html, plus more maintainable Completes #5
@@ -85,6 +85,9 @@ | |||
<ItemGroup> | |||
<EmbeddedResource Include="Transforms\HtmlSummary-v3.xslt" /> | |||
</ItemGroup> | |||
<ItemGroup> | |||
<Content Include="Transforms\HtmlTransform-v3.xslt" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be an EmbeddedResource
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, good spot. Updated
This is looking good. I think we should add in the command line options like we do on the command line. I think that will satisfy the where clause too. BTW, just saw your commit in response to my comment. You're quick 👍 |
@rprouse - should we add the command line in for all transforms, or just default and both html? |
@harrisonmeister I think we should add it to all of the full transforms and leave it out of the summaries. If we aren't putting errors in the summaries, I don't see the point of the command line, but then again, I don't really see much use in the summaries 😄 |
I've added the command line section to the HtmlSummary and the DefaultTransform for v3.
|
I figured we should have a general discussion of formatting before the release, so I created issue #9 for that purpose. Although it's not how we usually work, I think it may go faster if we finish up this issue, #6 and #2 before going back for a final review and comparison of everything. If nothing else, it will be a second chance to change what the output looks like. When we get to that point, I figured I would put up a side-by-side comparison for us to review. |
Ok, do you want me to combine the changes in this branch to #8 ? |
@harrisonmeister If a filter has been used, the My suggestion is to leave this out for now. We can discuss it along with #9 and possibly create an enhancement issue for post-1.0 working. |
@rprouse I created the summaries for my own use, so I can give you a use case. You have multiple XML result files and there is no external indication of which ones passed and which one's failed. You use the |
This looks good to me... merging. |
Cool thanks. I should be able to get to #8 early next week. |
@harrisonmeister We generally don't combine things, except in the rare case where two issues involve changing code in the same place. After a while, creating separate issues and PRs is second nature and doesn't seem like extra work. :-) |
No worries, and makes sense. I think I got confused as the PR got closed, and then merged :) |
As a future reference, we assume any PR is ready to be merged once reviewed and approved unless it's otherwise indicated. Sometimes we will create the PR early so we have a place for discussion or because work is still ongoing. In that case, we put a notice like Do Not Merge Yet at the top of the description. When we are ready to merge, we add a comment and edit the description. |
Thanks @CharliePoole for the info |
This provides output for HtmlTransform as below:
Also raw html output:
Questions