search

garris / BackstopJS

Catch CSS curve balls.
http://backstopjs.org
MIT License
6.82k stars 607 forks source link

Adding puppeteer should be a major version update #819

Open babielmam opened 6 years ago

babielmam commented 6 years ago

I support adding puppeteer. However, adding it crashed our CI process in the way, that Puppeteer requires at least Node version 6.4.0. Prior to adding Puppeteer you could use BackstopJS with at least Node version 4.8.7. This in my view represents a breaking change and according to semantic versioning the new version should be a major version update.

From the Semantic Versioning Docs

MAJOR version when you make incompatible API changes, MINOR version when you add functionality in a backwards-compatible manner

Maybe you should consider changing the version retrospectively and republish the different versions to npm. On the other hand, I know that it probably isn't worth the hassle :D Just wanted to let you know :)

garris commented 6 years ago

Sure -- good point -- but does backstop fail to run under chromy or phantom?

babielmam commented 6 years ago

The problem occurs during npm install, because the Puppeteer installation fails on Node versions < 6.4.0. So it doesn't even come to running any of the available options.

garris commented 6 years ago

oh, well then yeah, you're totally right. Sorry bout that.