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

[main] enable css output without html output #296

Merged
merged 2 commits into from
Feb 21, 2017
Merged

[main] enable css output without html output #296

merged 2 commits into from
Feb 21, 2017

Conversation

pkra
Copy link
Contributor

@pkra pkra commented Feb 16, 2017

Resolves #292

@dpvc
Copy link
Member

dpvc commented Feb 21, 2017

I think the test should also check if the svg element is produced (one of the two changes relates to that). I would also say that the tests should also check if only the desired properties are produced. For example speakText appears in the result whether you have asked for it or not.

@dpvc
Copy link
Member

dpvc commented Feb 21, 2017

Other than that, the PR is good.

@pkra
Copy link
Contributor Author

pkra commented Feb 21, 2017

Thanks! I'll update the test.

@pkra
Copy link
Contributor Author

pkra commented Feb 21, 2017

I would also say that the tests should also check if only the desired properties are produced.

That will be covered by #285, correct?

@dpvc
Copy link
Member

dpvc commented Feb 21, 2017

That will be covered by #285, correct?

It would be if that were implemented, yes. But no one commented on it, so I was not sure if anyone thought that was a good idea.

@pkra
Copy link
Contributor Author

pkra commented Feb 21, 2017

But no one commented on it

Copy that. Just added +1.

Also, I've updated the test. PTAL?

@dpvc
Copy link
Member

dpvc commented Feb 21, 2017

OK, looks good.

@pkra pkra merged commit aa4f2b1 into develop Feb 21, 2017
@pkra pkra deleted the issue292 branch February 21, 2017 15:47
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.

2 participants