Package home | Report new bug | New search | Development Roadmap Status: Open | Feedback | All | Closed Since Version 2.0.0RC3

Request #12380 add exitcodes on errors and check PHP version
Submitted: 2007-11-04 10:53 UTC Modified: 2007-11-05 18:19 UTC
From: cwiedmann Assigned: squiz
Status: Closed Package: PHP_CodeSniffer (version CVS)
PHP Version: Irrelevant OS: Irrelevant
Roadmaps: 1.0.0RC2    
Subscription  


 [2007-11-04 10:53 UTC] cwiedmann (Carsten Wiedmann)
Description: ------------ Hello, a) I think, the script should allways return an exitcode of 1, if an error occurs. b) I know, the PEAR installer checks the PHP version dependence during installation of a package. In my case, I often change the active PHP version during writing/testing scripts. So it would be nice, the phpcs script checks also the PHP version at start. c) fix some linelenght notices. Regards, Carsten

Comments

 [2007-11-04 17:04 UTC] squiz (Greg Sherwood)
For a), the phpcs script already does return an exitcode of 1 if any errors are found, a 0 if no errors are found. Was added for bug #10348.
 [2007-11-04 17:05 UTC] squiz (Greg Sherwood)
For b), what exactly could I be checking?
 [2007-11-04 17:06 UTC] squiz (Greg Sherwood)
For c), which notices do you want me to fix?
 [2007-11-04 17:09 UTC] squiz (Greg Sherwood)
Ignore me, I just saw the patch. Missed the email for some reason.
 [2007-11-04 17:14 UTC] squiz (Greg Sherwood)
My new comments: For a) I think returning 1 both on errors found and on incorrect script usage is not correct. If the file could not be checked, a different exitcode would be required. However I do think returning an exitcode is a good idea, so thanks for the patch. For b) I guess this is fair enough for a command line script. Thanks for the patch. For c) I don't correct any line length errors in code (I do for comments) because there is not clearly defined standard for how to do it. I will change them if and when PEAR defines how it should be done.
 [2007-11-04 22:51 UTC] squiz (Greg Sherwood)
This bug has been fixed in CVS. If this was a documentation problem, the fix will appear on pear.php.net by the end of next Sunday (CET). If this was a problem with the pear.php.net website, the change should be live shortly. Otherwise, the fix will appear in the package's next release. Thank you for the report and for helping us make PEAR better. phpcs script nows check the php version each time it is run and exits with status 2 if it invalid input was supplied.
 [2007-11-05 06:42 UTC] cwiedmann (Carsten Wiedmann)
Hi Greg, > For c) I don't correct any line length errors in code That's ok, it's your code ;-) (I consider myself just to the defined rule: "line lenght < 85". And how is my decision, because not defined). One addition to something I've just saw in the output of line 59 (phpcli): Assuming a default terminal width of 80, I think it's (looks) better to add a PHP_EOL and some spaces before: "summary" report Regards, Carsten
 [2007-11-05 18:19 UTC] squiz (Greg Sherwood)
Fair point about the 80 chars, and now fixed in CVS.