Collection of hacks for PHP 8 batch compatibility scanning #306
Conversation
There was a problem hiding this comment.
Code analysis identified issues
VIP Code Analysis Bot has identified potential problems in this pull request during automated scanning. We recommend reviewing the issues noted and that they are resolved.
phpcs scanning turned up:
🚫 10 errors
This bot provides automated PHP linting and PHPCS scanning. For more information about the bot and available customizations, see our documentation.
Scan run detail
Software versions
Options file (
|
PHP lint optionsPHP lint files enabled:
Lint modified files only:
Directories not PHP linted:
SVG configurationSVG scanning enabled:
Auto-approval configurationAuto-approvals enabled:
Non-functional changes auto-approved:
Auto-approved file-types:
|
PHPCS configurationPHPCS scanning enabled:
PHPCS severity level:
Standard(s) used:
Runtime set:
Custom sniffs included:
Custom sniffs excluded:
Directories not PHPCS scanned:
WPScan API configurationWPScan API scanning enabled:
|
| @@ -37,6 +37,9 @@ function vipgoci_log( | |||
| * otherwise, do print it, | |||
| */ | |||
|
|
|||
| // HACK | |||
There was a problem hiding this comment.
🚫 Error( severity 5 ): Inline comments must end in full-stops, exclamation marks, or question marks (Squiz.Commenting.InlineComment.InvalidEndChar).
| ); | ||
| // Hack - we don't need to copy over the file, we can just pass the contents | ||
| // $temp_file_name = vipgoci_save_temp_file( | ||
| // 'vipgoci-phpcs-scan-', |
There was a problem hiding this comment.
🚫 Error( severity 5 ): Expected 1 space before comment text but found 2; use block comment if you need indentation (Squiz.Commenting.InlineComment.SpacingBefore).
| // Hack - we don't need to copy over the file, we can just pass the contents | ||
| // $temp_file_name = vipgoci_save_temp_file( | ||
| // 'vipgoci-phpcs-scan-', | ||
| // $file_extension, |
There was a problem hiding this comment.
🚫 Error( severity 5 ): Expected 1 space before comment text but found 2; use block comment if you need indentation (Squiz.Commenting.InlineComment.SpacingBefore).
| // $temp_file_name = vipgoci_save_temp_file( | ||
| // 'vipgoci-phpcs-scan-', | ||
| // $file_extension, | ||
| // $file_contents |
There was a problem hiding this comment.
🚫 Error( severity 5 ): Expected 1 space before comment text but found 2; use block comment if you need indentation (Squiz.Commenting.InlineComment.SpacingBefore).
| // 'vipgoci-phpcs-scan-', | ||
| // $file_extension, | ||
| // $file_contents | ||
| // ); |
There was a problem hiding this comment.
🚫 Error( severity 5 ): Inline comments must end in full-stops, exclamation marks, or question marks (Squiz.Commenting.InlineComment.InvalidEndChar).
|
|
||
| unlink( $temp_file_name ); | ||
| // HACK | ||
| // unlink( $temp_file_name ); |
There was a problem hiding this comment.
🚫 Error( severity 5 ): Inline comments must end in full-stops, exclamation marks, or question marks (Squiz.Commenting.InlineComment.InvalidEndChar).
| $file_issues_arr_master['totals'] : null, | ||
| ) | ||
| ); | ||
| // HACK |
There was a problem hiding this comment.
🚫 Error( severity 5 ): Inline comments must end in full-stops, exclamation marks, or question marks (Squiz.Commenting.InlineComment.InvalidEndChar).
|
|
||
| // HACK |
There was a problem hiding this comment.
🚫 Error( severity 5 ): Inline comments must end in full-stops, exclamation marks, or question marks (Squiz.Commenting.InlineComment.InvalidEndChar).
| /* Get rid of temporary file */ | ||
| unlink( $temp_file_name ); | ||
| // unlink( $temp_file_name ); |
There was a problem hiding this comment.
🚫 Error( severity 5 ): Inline comments must end in full-stops, exclamation marks, or question marks (Squiz.Commenting.InlineComment.InvalidEndChar).
| /* Get rid of temporary file */ | ||
| unlink( $temp_file_name ); | ||
| // unlink( $temp_file_name ); |
There was a problem hiding this comment.
|
Hi, Thanks for submitting this. I will have look and evaluate each change. |
|
I will be working through this item by item 🙂
How about that each log message has an associated log level? The levels would be:
What do you think? |
I like that, it seems to make the most sense and allows us the necessary flexibility depending on the use case. |
This is resolved via #316 and Automattic/vip-go-compatibility-scanner#44 |
I don't expect this ever to get merged, but maybe some of the things done can be worked in the codebase properly.
When running the scans, we have had to deal with a few issues:
To achieve better concurrency we had to reduce the number of IO operations.
I spotted that vip-go-ci copies over the file to a temporary file and then scans that - I'm not sure of the reason, but I don't think it's relevant when running locally. So I just circumvented the part.
Log verbosity
Had to tweak log levels to reduce the amount of information, so that only the errors are printed.
For example, the entry is not printed when the scan result is empty.
Increase intervals
GH Issues API is throttling aggressively with "secondary rate-limits". To account for that, the
VIPGOCI_HTTP_API_WAIT_TIME_SECONDSwas bumped to 5.--error-severity vs --warning-severity
The production version uses
--severity=5which means--error-severity=5 --warning-severity=5. However, I discovered that having--severity=5trumps both specific arguments.