Skip to content
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

Added new htmlextra output #57

Merged
merged 6 commits into from
May 21, 2019

Conversation

Scott-Emberson
Copy link
Contributor

@sebcaps I have attempted to add the commands for the htmlextra options. #56

@carlowahlstedt
Copy link
Owner

@Scott-Emberson this is awesome! I love seeing more support, especially because my job has gotten so hectic.

I’ll look this over when I get a chance and I’m sure @sebcaps will as well. Two things I see just glancing over on my iPad is that you might be requiring the path on the new field (which I’m thinking we’ll likely not want to do) and the task.json looks like it probably got auto formatted which makes it difficult to tell what actually changed.

@sebcaps
Copy link
Contributor

sebcaps commented May 8, 2019

Sounds good to me with the following remarks:

  • I think you should add a condition to display html extra option only if this report type is selected (add visibleRule )
  • as this report need an extra lib , we should try to install it at least verify it's installed and update readme to add it as a requisite.

Good job anyway 😀

Copy link
Contributor Author

@Scott-Emberson Scott-Emberson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completed

@Scott-Emberson
Copy link
Contributor Author

Sorry do I need to do anything more on this pull request (it's my first one)

@sebcaps
Copy link
Contributor

sebcaps commented May 11, 2019

@Scott-Emberson, before merging the PR can you just have a look to my comment about htmlExtraReportTitle I can't see the update.
Then nothing special to do, just need to wait that @carlowahlstedt have some time to perform the merge.
Thx for your help improving this plugin

@DannyDainton
Copy link

DannyDainton commented May 15, 2019

Thanks for including the htmlextra reporter ❤️

Not sure if this is useful or relevant in this context but I've turned off the console.log() output on the report by default and included another other option to enable these.

@carlowahlstedt
Copy link
Owner

Just getting back to this. The main issue I'm having in the compare is the tabs vs spaces change in the task.json file. I'll look through this and try to find the differences.

@carlowahlstedt carlowahlstedt merged commit 76b8ce9 into carlowahlstedt:master May 21, 2019
@carlowahlstedt
Copy link
Owner

Turns out to be easier than I expected. Looks good. @sebcaps any reason you can think of why this wouldn't be backward compatible?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants