Problem/Motivation

Upgrade to https://github.com/mglaman/phpstan-drupal/releases/tag/1.1.21 to allow testing PHP 8.2.

The fix https://github.com/mglaman/phpstan-drupal/commit/8acaa1e4a15b266cfbd404c... is required to allow phpStan to run on core

OTOH CI run still fails

Steps to reproduce

See https://www.drupal.org/pift-ci-job/2420734

Proposed resolution

- decide why CI still broken, probably because of composer version
- asked for upgrade https://drupal.slack.com/archives/C033S2JUMLJ/p1657740913314079
- upgrade to latest release

Remaining tasks

- find a cause
- patch/commit

User interface changes

no

API changes

no

Data model changes

no

Release notes snippet

Comments

Spokje created an issue. See original summary.

spokje’s picture

Issue summary: View changes
spokje’s picture

StatusFileSize
new2.38 KB

Can it be this easy?

spokje’s picture

Status: Active » Closed (duplicate)
spokje’s picture

Assigned: spokje » Unassigned
spokje’s picture

andypost’s picture

I think we should raise requirement if we will declare compatibility

And this is not duplicate, patch testing issue is not supposed to be commited

andypost’s picture

Title: Update mglaman/phpstan-drupal to 1.1.25 » Update mglaman/phpstan-drupal to 1.1.25 to unblock testing on PHP 8.2
Issue summary: View changes

Updated IS to point why this still makes sense

andypost’s picture

Assigned: Unassigned » xjm

I recall @xjm already pointed that this upgrade can cause a lot of new warnings, assigning to her to call details

longwave’s picture

Status: Needs review » Needs work

Baseline needs regenerating because this upgrade solves the false positives in #3259716: Replace usages of static::class . '::methodName' to first-class callable syntax static::method(...)

mondrake’s picture

#3295618: Get PHPStan baseline generation via DrupalCI would help in regeneration of baseline as part of patches preparation.

andypost’s picture

Thank you but I'm afk til evening

spokje’s picture

Assigned: xjm » spokje
spokje’s picture

StatusFileSize
new7.55 KB
new3.65 KB

I think we should raise requirement if we will declare compatibility

I'm always insecure about when to do this, to me it seems quite random ATM: For example: we didn't do it for the update for phpstan itself and don't seem to be doing it for the composer/composer update in another child issue.

Anyway: rebuild the phpstan baseline (vendor/bin/phpstan analyze --configuration=core/phpstan.neon.dist --generate-baseline ./core/phpstan-baseline.neon)

andypost’s picture

Status: Needs work » Needs review

Queued for 8.2 too

spokje’s picture

Assigned: spokje » Unassigned
Status: Needs review » Needs work
spokje’s picture

Status: Needs work » Needs review
mondrake’s picture

Re #14: IMHO we should bump the version requirements when the upgrade to a new version require changes to the baseline. Like here. Otherwise running the analysis on older version may throw errors.

spokje’s picture

Thanks @mondrake for explaining, makes sense

mondrake’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me, thanks

andypost’s picture

  • catch committed 265b3c9 on 10.1.x
    Issue #3299606 by Spokje, andypost, mondrake, longwave: Update mglaman/...

  • catch committed 4c29de4 on 10.0.x
    Issue #3299606 by Spokje, andypost, mondrake, longwave: Update mglaman/...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 10.1.x and cherry-picked to 10.0.x, thanks!

xjm’s picture

Issue tags: +10.0.0 release notes

Does this also need to be backported to 9.5.x which should also presumably be made compatible with PHP 8.2? Edit: Looks like we don't have this dependency there, which is slightly confusing.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.