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

Add Test Coverage for --bundle and --consolify args #141

Merged
merged 1 commit into from
Jul 15, 2016

Conversation

jonnyreeves
Copy link
Contributor

@jonnyreeves jonnyreeves commented Jul 14, 2016

  • Fail with a useful error if the user tries to use --bundle wihout --consolify
  • Add test-coverage to check that both the consolify'd HTML and extracted bundle are written to disk

@mantoni
Copy link
Owner

mantoni commented Jul 14, 2016

Hey, thanks! One thing: The bundle option is, well, optional :) If it's not provided, the bundle is inlined into the html file. Can I ask you to ensure this continues to work?

@jonnyreeves
Copy link
Contributor Author

Ah sorry, I forgot to test that flow! Let me patch that up.

On Thu, 14 Jul 2016, 21:52 Maximilian Antoni, [email protected]
wrote:

Hey, thanks! One thing: The bundle option is, well, optional :) If it's
not provided, the bundle is inlined into the html file. Can I ask you to
ensure this continues to work?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#141 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAMN-Qe3Vv__ISyBQiDW_qRuYppt2aiFks5qVqGCgaJpZM4JM1D4
.

* Fail with a useful error if the user tries to use --bundle wihout --consolify
* Add test-coverage to check that both the consolify'd HTML and extracted bundle are written to disk
@jonnyreeves jonnyreeves changed the title Resolve relative path to external bundle when passing to consolify Add Test Coverage for --bundle and --consolify args Jul 15, 2016
@jonnyreeves
Copy link
Contributor Author

Hey @mantoni; so in adding test coverage I realised that this was the wrong place to make this fix - I'll raise a PR against consolify to ensure the bundle is still written to the correct location, just the script src value is changed to be relative.

Cheers

@@ -43,7 +43,6 @@ rules:
no-return-assign: 2
no-self-compare: 2
no-spaced-func: 2
no-sync: 2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could change this so it only affects tests if you would prefer?

@mantoni mantoni merged commit 13a8ac4 into mantoni:master Jul 15, 2016
@mantoni
Copy link
Owner

mantoni commented Jul 15, 2016

Awesome! Thanks a lot 👍

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