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

[REF][PHP8.1] Apply patches from upstream to ensure league/csv can work on php8.1 #24046

Merged
merged 2 commits into from
Jul 25, 2022

Conversation

seamuslee001
Copy link
Contributor

Overview

Note that this incldues the PR #24045

This applies the following patches (one minorly modified locally) to get league/csv to work with php8.1

1
2
3
4
5

Before

League/csv installed version won't work with php8.1

After

League/csv version does work with php8.1

If we just upgraded league/csv to the released version that contained these patches we would need to bump our minimum install version to php7.3 which might be a reasonable thing to do anyway but this at least allows us to get through without needing to do that

ping @demeritcowboy @totten @eileenmcnaughton @colemanw

@civibot
Copy link

civibot bot commented Jul 24, 2022

(Standard links)

@civibot civibot bot added the master label Jul 24, 2022
@eileenmcnaughton
Copy link
Contributor

OK -our usage of csv is not complicated and has test cover and there is also test cover in the wmf codebase so I feel good about our test cover. In WMF codebase we hit incompatibilitiies last upgrade - which showed that the tests work (the fix was a composer update to wmf code).

@eileenmcnaughton eileenmcnaughton merged commit d073132 into civicrm:master Jul 25, 2022
@eileenmcnaughton eileenmcnaughton deleted the league_csv_php81 branch July 25, 2022 09:39
@demeritcowboy
Copy link
Contributor

Patches don't seem to work in drupal 9: https://lab.civicrm.org/dev/core/-/issues/3766

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants