DASPRiD / Flitch

PHP Coding Standard Validator
BSD 2-Clause "Simplified" License
58 stars 9 forks source link

./flitch exit code #2

Open drdev opened 12 years ago

drdev commented 12 years ago

In order to effectively use the script we need to add exit code. What's the best way to enumerate number of errors (not violations!)?

DASPRiD commented 12 years ago

We should first decide, which kind of script errors we have to supply exit codes for. Could you compile a list?

drdev commented 12 years ago

In fact the only one I am interested in now is whether there were errors in files being checked. "0" for success, "1" for failed check. Internal errors' handling is not the purpose of this request.

I guess we can also provide an option to specify minimum severity level for check, and return fail code when one above that value happened.

DASPRiD commented 12 years ago

Well, i don't know if this conforms with the unix style. 0 usually means the application exited without errors, while anything else means "there was an error while executing the command". When you'd use this in a Makefile, make would abort as soon as a cs violation is found. I don't know if we really want that.

drdev commented 12 years ago

It's the only way you can integrate it into existing CI environments. So, not much choice here.

Personally I consider returning none zero exit code is fine. And yes, it conforms Unix way.

DASPRiD commented 12 years ago

How about we follow the checkstyle way then, and use the count of "error" violations as exit code? See: http://checkstyle.2069334.n4.nabble.com/exit-code-of-checkstyle-not-zero-td2532939.html

drdev commented 12 years ago

Exactly, though PHPCS itself uses$num > 0 condition...

Maybe just return 0/1, and leave rest for internal errors reporting (not useful though, imho). We can also produce summary report, but that's not the purpose of the issue.

DASPRiD commented 12 years ago

I think it's better to follow the checkstyle example, as that allows to use Flitch output the same way as checkstyle.

drdev commented 12 years ago

In fact we don't have to follow Checkstyle convention, as we'll have (probably) many reports. Can't just follow each convention. So we have to choose our own. The last word is yours, I am ready to implement it (just reply the question in topic ;) )

DASPRiD commented 12 years ago

Yeah but checkstyle is most implemented in the CI systems ;)…

I think we can just iterate over $file->getViolations() after each analyzeFile() and count the number of violations which are errors. Sum that up at the end and use it as exit code. I actually like the checkstyle idea myself, because the higher the exit code, the more broken your code is ;)

By the way, in Flitch\File\File, could you modify getViolations() to only usort() when required? (let addViolation() set some internal $sorted flag to false)?