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

feat: @PhpCsFixer ruleset - enable multiline_string_to_heredoc #7706

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

mvorisek
Copy link
Contributor

@mvorisek mvorisek commented Jan 10, 2024

the fixer was introduced in #7665

the code is now much readable, especially test cases where multiline strings were used often

@Wirone
Copy link
Member

Wirone commented Jan 10, 2024

+74,844 −58,592 😵. That's probably too big PR to handle.

@mvorisek mvorisek force-pushed the convert_all_multiline_strings_to_heredoc branch from 0830301 to 1e0f8f2 Compare January 12, 2024 16:05
@mvorisek
Copy link
Contributor Author

mvorisek commented Jan 12, 2024

+74,844 −58,592 😵. That's probably too big PR to handle.

@Wirone instead of landing #7709 first I decided to help with the review as much as possible, I changed no_trailing_whitespace_in_string fixer temporary to actually keep the trailing whitespaces that it would remove after multiline_string_to_heredoc applied normally.

The changes made in this PR should be solely CS. All strings should be effectively unchanged. Later, we can land improvements like in #7709, but such changes will change the strings, so let's not mix them in this large PR.

Can I please ask you to review this PR asap to prevent conflicts?

Here is how to review it:

  1. checkout this PR branch
  2. reset your local branch to the 1st commit
  3. run fixer locally and verify if all the changes are equal to the 2nd commit (run fixer)
  4. do 2nd/3rd step also for the next two run fixer commits

@mvorisek mvorisek marked this pull request as ready for review January 12, 2024 16:10
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.

None yet

3 participants