Svgo: Broken CLI Thanks To COA & JavaScript Hell
Looking through your source, it seems like you use this thing to handle your CLI option parsing.
I just reinstalled SVGO 0.7.2 and npm pulled down COA 1.0.2. This seems to have introduced the following bug:
The Bug
Running svgo -v at the command line will now return an exit code of 1. This may not SEEM like a big deal, but if you happen to be integrating SVGO into another tool, a non-zero exit code means, "Something went wrong; the program did not run successfully." The correct exit code is super important and, here, it should absolutely be 0 because SVGO successfully returned a version.
It's Definitely COA
The reason I think this is COA's fault is that you already have a workaround in your code to log Version output to stdout instead of stderr and a comment indicating this is a workaround for COA.
Additionally, an older installation of SVGO 0.7.2 works just fine and that installation has COA 1.0.1. When I used COA 1.0.1 in the new installation, the problem went away.
Discussion
I'm not sure what's changed, exactly, but it looks like COA wasn't under active development for a while and just pushed through a bunch of changes (looking at their commit history).
Perhaps lock the dependency to COA 1.0.1? Or move to a different library? Or perhaps your workaround is no longer needed with COA 1.0.2 because they cleaned up their end, so now your workaround is actually doing the reverse of what's correct? I dunno... it's 1AM here and I'm not JavaScript's biggest fan.
bdkjones
All 3 comments
Also, that workaround I mentioned is in coa.js here:
bdkjones
on 10 Jun 2017
Please try to reinstall svgo, this should be fixed in COA v1.0.3
qfox
on 10 Jun 2017
Workaround could be dropped now, it's true.
ps.
Looks like I should clarify what happened with COA and why it wasn't under active development.
COA was originally written on CoffeeScript and since nobody want to write code on Coffee there were no activity.
About a month ago we decided to rewrite COA to Node 4+ and ES6 classes because we love how COA was designed, it's API and we have a bunch of ideas to evolute.
Since we shouldn't change anything in the public API, we decided to publish it under 1.0.2.
Unfortunately COA has like 70% lines coverage and practically there are a lot of uncovered edge-cases.
Sorry that it affected you in CI like that ;-(
qfox
on 10 Jun 2017
Related issues
elidupuis
路
5Comments
madysondesigns
路
4Comments
niftylettuce
路
4Comments
ai
路
5Comments
crybat
路
6Comments
Most helpful comment
Workaround could be dropped now, it's true.
ps.
Looks like I should clarify what happened with COA and why it wasn't under active development.
COA was originally written on CoffeeScript and since nobody want to write code on Coffee there were no activity.
About a month ago we decided to rewrite COA to Node 4+ and ES6 classes because we love how COA was designed, it's API and we have a bunch of ideas to evolute.
Since we shouldn't change anything in the public API, we decided to publish it under 1.0.2.
Unfortunately COA has like 70% lines coverage and practically there are a lot of uncovered edge-cases.
Sorry that it affected you in CI like that ;-(