-
Notifications
You must be signed in to change notification settings - Fork 121
Add "debug" argument for straightforward debugging #43
Conversation
+1 |
4 similar comments
+1 |
+1 |
+1 |
+1 |
Thank you, I like this a lot but could you please do the following changes:
|
Ok I think I have addressed all three things, please let me know if I've done any of them incorrectly. |
" }\n" + | ||
" debugPage();\n" + | ||
"}(phantom));\n" + | ||
"\n"; |
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.
I'm not a big fan of having that much code in quotes, how about you put this into a separate file and then do something like this:
// capture.template
..
page.open('<%= url %>');
..
// ---
fs.readFile('./templates/capture.js', function (err, file) {
captureCode = _.template(file.toString(), {url: url})
})
I've moved the template code into its own file, added underscore.js and tested on Windows and Ubuntu. Let me know if there's anything else. |
Thanks, much better :) One last nitpick please use |
move debugPage code into a template file remove debug logging code move underscore from devDependencies to dependencies switch from underscore library to lodash
Done: underscore --> lodash |
LGTM |
I'm not sure if there's anything more I should do... I can't merge it myself, right? |
Add "debug" argument for straightforward debugging
I found it really difficult to debug tests with Karma and PhantomJS, and I saw others struggling too so I modified the code to make it easier. There may be better ways to do this but it worked for me and everything else seems really brittle.