- Issue created by @sime
- First commit to issue fork.
- Status changed to Needs work
10 months ago 10:39pm 4 March 2024 - 🇦🇺Australia mstrelan
Nice, the phpstan fail is because we are ignoring this error in the phpstan baseline and that is no longer necessary. That validates that adding this @property is useful not only downstream but in core also. Needs work to remove the ignore rule from the phpstan baseline.
- Status changed to Needs review
10 months ago 5:56am 5 March 2024 - Status changed to Needs work
10 months ago 6:14am 5 March 2024 - 🇦🇺Australia mstrelan
This fixes phpstan errors in
\Drupal\datetime\Plugin\Field\FieldType\DateTimeItem::onChange
that were in the baseline.I think we should update
\Drupal\datetime\Plugin\Field\FieldFormatter\DateTimePlainFormatter::viewElements
and other related formatters. It seems we're using the local variable$date
simply so we can typehint it to DrupalDateTime. I think instead we should typehint$item
toDateTimeItemInterface
and drop the local variable.Wondering though if
DateTimeItem::$date
could benull
though, in which case we need a union type on the@property
. - 🇦🇺Australia sime Melbourne
I'm wondering why we wouldn't protect the date property and add something like a getDate() method in that case (from a position of ignorance).
- 🇳🇿New Zealand quietone
Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.