More comprehensive scrutinizer config.#571
Conversation
jrfnl
force-pushed
the
feature/scrutinizer-config
branch
5 times, most recently
from
2e38827 to
f17ecd1
Compare
class-tgm-plugin-activation.php
Outdated
|
|
||
| $prefix = ( defined( 'WP_NETWORK_ADMIN' ) && WP_NETWORK_ADMIN ) ? 'network_admin_' : ''; | ||
| return apply_filters( "tgmpa_{$prefix}plugin_action_links", array_filter( $action_links ), $item['slug'], $item, $this->view_context ); | ||
| return apply_filters( 'tgmpa_' . $prefix . 'plugin_action_links', array_filter( $action_links ), $item['slug'], $item, $this->view_context ); |
There was a problem hiding this comment.
AFAIK, filters (and the action hook below) need to be named with interpolation, for WP-Parser to be able to read the single string. Unless WP-Parser has been updated to read the first arg token, instead of the first string token...
There was a problem hiding this comment.
Intresting. I didn't know that. Bit of a shame that they're promoting bad practices that way, but fair enough.
I'll try and figure out the rule which triggered this change and turn it off in the config.
There was a problem hiding this comment.
Also just wondering: we're not using the WP-Parser, so why should we be led by what it can parse ?
There was a problem hiding this comment.
While we're not, there may be plugins and themes that are using it, so TGMPA shouldn't be the cause of them getting duff auto-generated documentation because it tripped up in some way.
Of course, we could use WP-Parser if we wanted to create some technical API documentation.
There was a problem hiding this comment.
Fair point.
Of course, we could use WP-Parser if we wanted to create some technical API documentation.
I was considering something like that at some point once all the hooks are documented.
All the same, if the WP Parser is that finicky, we may want to consider using WP Hook documentor instead. At least it parses all variants of hooks ;-)
Or maybe better: fix WP Parser ... (yet another project to fix... sigh)
|
Probably should have been separate commits - the config file is fine to merge, but one small query on one of the fixes means the PR can't be merged immediately. |
|
See my above comment - exactly the reason why I put in one PR (though I can easily make it two commits of course). |
|
Are you planning to change these hook names back or not? |
|
Yes I am, just need to figure out the scrutinizer rule to invert. |
jrfnl
force-pushed
the
feature/scrutinizer-config
branch
from
47c6619 to
1ef8ab4
Compare
|
Adjusted & turned into two separate commits, one for the config, one for the code adjustments. The issue is triggered by the WPCS rules, so no specific Scrutinizer rule I can disable for that. In the WPCS config the severity is |
|
Hang on - some alignment issues were introduced by the translators comments, let me fix those too. |
jrfnl
force-pushed
the
feature/scrutinizer-config
branch
from
1ef8ab4 to
46b4485
Compare
|
Ok, all done. |
2 participants
Prevents builds in forks throwing different results (Scutinizer uses a config cascading merge).
Includes minor code changes based on Scrutinizer feedback.
Also reset the Scrutinizer filtered issues and only re-filtered persistent issues we can't currently get rid off for varying reasons.