respect the film-strip option #87
Conversation
|
Phantomjs is crashing on certain urls when the |
|
Hey. Okay thank you. Didn't notice that behavior. Could you please fix tests? After that we can release patch. :) |
|
Okay, I've changed it so that it's on by default unless you set film-strip to false. Yup, PhantomJS crashes on certain urls even just using phantomas itself with |
|
Great. Thanks a lot... will release patch today and make a note to readme. Is the error clear, when it happens in "If you see this error - Maybe you can share it? :) |
|
Sure thing, happy to, but the error is extremely non-descriptive. grunt phantomas crashes with |
|
Hmm... that's kind of disappointing. ;) Thanks for investigating. But if it's such a undescribtive and regular bug, why not switching film strip off as default. I mean, in my opinion stableness is more important than the images? |
|
The filmstrip feature is cool, so I like it by default. I'm trying to get it to error out more gracefully, rather than just die, but I haven't had much luck so far. I haven't tested enough URL's to be sure, but it might just be a problem in the page I am testing. @macbre has flagged the film-strip option as experimental, so I've added a Troubleshooting section in the README PS - great job with this plugin, it's awesome. Thanks so much. |
|
Great. I'm on a conference the next two days, so I won't be able to merge today. But looks good. Thanks a lot. Plan to merge and release new version during the week. :) |
|
👍 |
| @@ -308,7 +308,7 @@ Phantomas.prototype.executePhantomas = function() { | |||
| options = _.clone( this.options.options ); | |||
|
|
|||
| // run it only for the first run | |||
| if ( i === 0 ) { | |||
| if ( i === 0 && options['film-strip'] !== false) { | |||
| options[ 'film-strip' ] = true; | |||
There was a problem hiding this comment.
One tiny little thing.
Can I have three more spaces here?
Will merge after that. :)
And sorry for being picky on coding style...
There was a problem hiding this comment.
Haha, of course. NP, picky is good :)
|
By the way, our use case requires passing in parameters via the command line i.e. |
|
Thanks a lot for contribution. Let's discuss last question in #89. :) |
respect the film-strip option
No description provided.