-
Notifications
You must be signed in to change notification settings - Fork 13
N°8796 - Add PHP code style validation in iTop and extensions #62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Ew, spaces for indentation? 😱 Current docs luckily still say to use tabs. |
thanks for this feedback. good catch! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks much better now, some minor comments/suggestions.
| if (in_array($sHeader, $aField['columns'])) return true; | ||
| if (in_array($sHeader, $aField['columns'])) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Single line if-return should be allowed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice to have but tricky to configure
PHP-CS-Fixer/PHP-CS-Fixer#2780
| $sResult = self::CallItopViaHttp( | ||
| "/synchro/synchro_exec.php?login_mode=$sLoginform", | ||
| $aData | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about
| $sResult = self::CallItopViaHttp( | |
| "/synchro/synchro_exec.php?login_mode=$sLoginform", | |
| $aData | |
| ); | |
| $sResult = self::CallItopViaHttp("/synchro/synchro_exec.php?login_mode=$sLoginform", $aData); |
| Utils::Log(LOG_ERR, | ||
| "[".get_class($this)."] defaults section configuration is not correct. please see documentation."); | ||
| Utils::Log( | ||
| LOG_ERR, | ||
| "[".get_class($this)."] defaults section configuration is not correct. please see documentation." | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar here (and other Utils::Log calls), why can't it just be on 1 line? It's not that complicated to see what's happening?
Current PR aims at validating code style easily via php-cs-fixer.
this tool is also integrated into Jenkins pipeline along to phpstan/phpunit.
Jenkins steps have been reordered logically (code style/phpstan/phpunit).
report generated in Jenkins is in verbose. you can have it by calling:
whole code base has been re-formatted. style will be clean starting with this PR.