Remove unused imports; Use str_starts_with() function
#1681
Conversation
janschoenherr
force-pushed
the
patch-3
branch
from
4ad2cc1 to
5523664
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1681 +/- ##
============================================
- Coverage 50.31% 50.25% -0.07%
- Complexity 2722 2726 +4
============================================
Files 99 99
Lines 11316 11322 +6
============================================
- Hits 5694 5690 -4
- Misses 5622 5632 +10 ☔ View full report in Codecov by Sentry. |
| @@ -717,11 +715,11 @@ public function registerMetabox() { | |||
| protected function getCustomFields() { | |||
| // We need static types, because german month names dont't work for datepicker | |||
| $dateFormat = 'd/m/Y'; | |||
| if ( strpos( get_locale(), 'de_' ) !== false ) { | |||
| if ( str_starts_with( get_locale(), 'de_' ) ) { | |||
There was a problem hiding this comment.
Das ist gedoppelter Code. Lässt sich das irgendwie sinnvoll auslagern?
There was a problem hiding this comment.
Ich persönlich würde das nicht auslagern. Ist eine sehr einfache Operation und taucht auch nur 4 mal im Code auf. Bzw. 8 mal mit den Englischen Vorkommen.
| @@ -293,6 +293,6 @@ public function registerMetabox() { | |||
| foreach ($cmb->meta_box['fields'] as $metabox_field) { | |||
| $metabox_fields[$metabox_field['id']] = $metabox_field['name']; | |||
| } | |||
| Settings::updateOption('commonsbooking_settings_metaboxfields', $this->getPostType(), $metabox_fields); | |||
| Settings::updateOption('commonsbooking_settings_metaboxfields', static::getPostType(), $metabox_fields); | |||
There was a problem hiding this comment.
Das wird nicht aus einem statischen Kontext aufgerufen, warum dann nicht this verwenden?
There was a problem hiding this comment.
So wie ich late static binding in php verstehe, wäre das nur relevant wenn es abgeleitete Klassen von Item gibt, welchen wir erlauben wollen registerMetabox mit ihrer eigenen getPostType Implementierung aufzurufen. Das ist hier aber nicht der Fall, ggf. war das ein false positive von phpstan für @janschoenherr
There was a problem hiding this comment.
Also bestimmt hätten wir das eindeutiger definieren können, aber mit $this wird garantiert, dass die Methode getPostType z.b. auch mit einer nicht-statischen Methode in der aktuellen Klasse überschrieben werden kann. Zumindest habe ich das so verstanden. Wir brauchen das vielleicht nicht unbedingt, das verhindert aber vielleicht in Zukunft schwer nachvollziehbare Fehler.
There was a problem hiding this comment.
Bei mir ist das eine Warnung in phpstorm. Ich finde es persönlich eher verwirrend, wenn statische Methoden nicht statisch aufgerufen werden. Als Alternative könnte man auch $this::getPostType() schreiben.
Ich kann das aber auch gern wieder zurück ändern.
There was a problem hiding this comment.
@hansmorb das von dir beschriebene Verhalten funktioniert auch mit static::. Die bisherigen Aufrufe der statischen Methode CustomPostType::getPostType geschehen auch auf statische Art und Weise.
Daher wäre ich doch auch für diese Änderung, hin zu dem static-Aufruf.
| if ( $cb_map_settings->get_option( 'booking_page_link_replacement' ) ) { | ||
|
|
||
| if ( MapSettings::get_option( 'booking_page_link_replacement' ) ) { |
There was a problem hiding this comment.
Zurückstellen für #1413 bitte.
| @@ -404,11 +404,11 @@ public function registerMetabox() { | |||
| protected function getCustomFields(): array { | |||
| // We need static types, because german month names dont't work for datepicker | |||
| $dateFormat = "d/m/Y"; | |||
| if ( strpos( get_locale(), 'de_' ) !== false ) { | |||
| if ( str_starts_with( get_locale(), 'de_' ) ) { | |||
There was a problem hiding this comment.
Gleiche Codedopplung, s.o.
| @@ -466,11 +465,11 @@ public function registerMetabox() { | |||
| protected function getCustomFields() { | |||
| // We need static types, because german month names dont't work for datepicker | |||
| $dateFormat = "d/m/Y"; | |||
| if ( strpos( get_locale(), 'de_' ) !== false ) { | |||
| if ( str_starts_with( get_locale(), 'de_' ) ) { | |||
janschoenherr
force-pushed
the
patch-3
branch
2 times, most recently
from
dfd2c08 to
bd2f85d
Compare
Use `str_starts_with()` function
janschoenherr
force-pushed
the
patch-3
branch
from
bd2f85d to
7bdad35
Compare
Just a bit of code cleanup.
EDITED by hansmorb:
str_starts_with is polyfilled by WP since 5.9