-
Notifications
You must be signed in to change notification settings - Fork 10
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
PES-2296, Check phpcs sniff ValidVariableName #625
base: main
Are you sure you want to change the base?
Conversation
c911f11
to
e1c76fe
Compare
if ( isset( $wp->query_vars['order-pay'] ) && is_numeric( $wp->query_vars['order-pay'] ) ) { | ||
$order = $this->orderRepository->getById( (int) $wp->query_vars['order-pay'], true ); | ||
if ( isset( $wp->query_vars['order-pay'] ) && is_numeric( $wp->query_vars['order-pay'] ) ) { // phpcs:ignore Squiz.NamingConventions.ValidVariableName.MemberNotCamelCaps | ||
$order = $this->orderRepository->getById( (int) $wp->query_vars['order-pay'], true ); // phpcs:ignore Squiz.NamingConventions.ValidVariableName.MemberNotCamelCaps |
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.
proč tady to ignorujeme? Takhle to nikdo nikdy neopraví. nejde to přesunou t do adapteru?
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.
To přichází z vnějšku, takže ignorování je ok. Ale bylo by asi přehlednější $wp->query_vars['order-pay']
vytáhnout ternárem do proměnné, pak tam i to ignorování bude jen jednou.
<arg name="encoding" value="utf-8"/> | ||
<arg value="p"/> | ||
<arg value="s"/> | ||
<description>Packetery WP plugin coding standards ruleset.</description> |
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.
Duplicitní řádek.
@@ -25,42 +25,42 @@ class WpdbAdapter { | |||
* | |||
* @var string | |||
*/ | |||
public $packetery_carrier; | |||
public $packeteryCarrier; |
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.
Ve wpdb je konvence, že název property odpovídá názvu tabulky. Raději bych s týmem ověřil, že se na camelCase pro toto použití shodneme.
@@ -181,8 +181,8 @@ public function check(): void { | |||
} | |||
|
|||
if ( $oldVersion && version_compare( $oldVersion, '1.4.2', '<' ) ) { | |||
$version_1_4_2 = new Version_1_4_2( $this->wpdbAdapter ); | |||
$version_1_4_2->run(); | |||
$version142 = new Version_1_4_2( $this->wpdbAdapter ); |
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.
To možná raději bez čísla verze. Stejně se tento způsob používá jen jednou.
if ( isset( $wp->query_vars['order-pay'] ) && is_numeric( $wp->query_vars['order-pay'] ) ) { | ||
$order = $this->orderRepository->getById( (int) $wp->query_vars['order-pay'], true ); | ||
if ( isset( $wp->query_vars['order-pay'] ) && is_numeric( $wp->query_vars['order-pay'] ) ) { // phpcs:ignore Squiz.NamingConventions.ValidVariableName.MemberNotCamelCaps | ||
$order = $this->orderRepository->getById( (int) $wp->query_vars['order-pay'], true ); // phpcs:ignore Squiz.NamingConventions.ValidVariableName.MemberNotCamelCaps |
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.
To přichází z vnějšku, takže ignorování je ok. Ale bylo by asi přehlednější $wp->query_vars['order-pay']
vytáhnout ternárem do proměnné, pak tam i to ignorování bude jen jednou.
@@ -237,7 +237,7 @@ private function formatVariable( $variable, int $level = 0, bool $addSeparator = | |||
} elseif ( $variable instanceof \WC_Shipping_Method ) { | |||
$methodInfo = [ | |||
'id' => $variable->id, | |||
'method_title' => $variable->method_title, | |||
'method_title' => $variable->method_title, // phpcs:ignore Squiz.NamingConventions.ValidVariableName.MemberNotCamelCaps |
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.
Ignorovací komentáře bych uváděl vždy nad řádek, usnadnilo by to porovnávání revizí.
No description provided.