Skip to content
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

fix: phpstan issues #288

Merged
merged 3 commits into from
Oct 2, 2023
Merged

fix: phpstan issues #288

merged 3 commits into from
Oct 2, 2023

Conversation

millnut
Copy link
Member

@millnut millnut commented Sep 29, 2023

Fix issues reported in #287

@@ -84,7 +84,7 @@ protected function prepareRevertedRevision(AlertBannerEntityInterface $revision,
$revert_untranslated_fields = $form_state->getValue('revert_untranslated_fields');

/** @var \Drupal\localgov_alert_banner\Entity\AlertBannerEntityInterface $default_revision */
$latest_revision = $this->AlertBannerEntityStorage->load($revision->id());
$latest_revision = $this->alertBannerEntityStorage->load($revision->id());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Casing fix as AlertBannerEntityRevisionRevertForm has this as alertBannerEntityStorage not AlertBannerEntityStorage

Copy link
Member

@finnlewis finnlewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests pass and looks good to me, thank @millnut !

Happy to have second opinion from @andybroomfield if needed.

@millnut millnut linked an issue Oct 2, 2023 that may be closed by this pull request
Copy link
Contributor

@andybroomfield andybroomfield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All seems sensible, and tested the functionality of the banner still works.

@andybroomfield
Copy link
Contributor

As an aside, the alert banner entity was genereted with an older version of the Drush entity generate plugin, and before that it used ECK, so we might want to look at regenerting that on the modern version later.

@finnlewis finnlewis merged commit 93f9a15 into 1.x Oct 2, 2023
8 checks passed
@finnlewis finnlewis deleted the fix/287-php-phpstan-issues branch October 2, 2023 13:17
@andybroomfield andybroomfield mentioned this pull request Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drupal 10, PHP 8.2 and PHPStan issues
3 participants