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: added PHP 8.2 to workflow tests #328

Merged
merged 3 commits into from
Dec 11, 2023
Merged

Conversation

millnut
Copy link
Member

@millnut millnut commented Nov 27, 2023

I noticed we are not currently checking PHP 8.2 for this module so I've added it to the workflow file.

Added PHP 8.2 compatibility patch for drupal/facets

@millnut
Copy link
Member Author

millnut commented Nov 28, 2023

Looking further at drupal/facets it is used across some other localgov modules so it might be preferred to have the patch in the profile

@finnlewis
Copy link
Member

Error in PHP 8.2:

1) Drupal\Tests\localgov_directories\Functional\FacetsTest::testFacetsGroupFilters
Exception: Deprecated function: Creation of dynamic property Drupal\facets\Result\Result::$transliterateDisplayValue is deprecated
Drupal\facets\Plugin\facets\processor\DisplayValueWidgetOrderProcessor->sortResults()() (Line: 72)

@Adnan-cds thinks he's seen this one.

@millnut
Copy link
Member Author

millnut commented Dec 4, 2023

@finnlewis yep for some reason the patch doesn't look like it is applying here correctly with the workflow. That error should be fixed by the drupal/facets patch; however I think I might revert that and put it in the profile as there are other modules which depend on drupal/facets and keep this to a workflow change for PHP 8.2 support only

@finnlewis
Copy link
Member

Noted. thanks @millnut

We should probably use the patch, rather than the merge request:

https://www.drupal.org/files/issues/2023-08-16/3336646-function-is-deprecated.patch

Currently it is the same, but a merge request can change.

@finnlewis
Copy link
Member

Going to try merging this and test on a release merge request

@finnlewis finnlewis merged commit cd66c05 into 3.x Dec 11, 2023
7 of 8 checks passed
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.

2 participants