Skip to content

[Serializer] remove custom CSV escape character from tests #58021

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

Merged
merged 1 commit into from
Aug 16, 2024

Conversation

xabbuh
Copy link
Member

@xabbuh xabbuh commented Aug 16, 2024

Q A
Branch? 5.4
Bug fix? no
New feature? no
Deprecations? no
Issues
License MIT

using custom escape character will be deprecated from PHP 8.4

@nicolas-grekas
Copy link
Member

Do we keep one as @group legacy for testing purposes?

@xabbuh
Copy link
Member Author

xabbuh commented Aug 16, 2024

none of the tests actually depends on the value of the escape character, not sure if we want to add a new test covering it explicitly

Copy link
Member

@chalasr chalasr left a comment

Choose a reason for hiding this comment

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

Should we deprecate the CsvEncoder::ESCAPE_CHAR_KEYcontext key on 7.2?

@nicolas-grekas
Copy link
Member

@chalasr see #57827

@xabbuh
Copy link
Member Author

xabbuh commented Aug 16, 2024

@chalasr I think @alexandre-daubois should have that covered in #57827

@alexandre-daubois
Copy link
Member

Indeed, I'll address the remaining comment in a couple of days. Thanks for this complementary PR!

@nicolas-grekas
Copy link
Member

Isn't the failing test related?

@xabbuh
Copy link
Member Author

xabbuh commented Aug 16, 2024

indeed, fixed

@nicolas-grekas
Copy link
Member

Thank you @xabbuh.

@nicolas-grekas nicolas-grekas merged commit 9348c49 into symfony:5.4 Aug 16, 2024
11 of 12 checks passed
@xabbuh xabbuh deleted the php-8.4-csv branch August 16, 2024 10:40
fabpot added a commit that referenced this pull request Aug 17, 2024
This PR was merged into the 6.4 branch.

Discussion
----------

[Serializer] clean up PHP version checks

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        |
| License       | MIT

these checks are not needed after merging #58021 up into the `6.4` branch

Commits
-------

6e19c0f clean up PHP version checks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants