Skip to content

[TASK] Avoid heredoc in another test#1411

Merged
JakeQZ merged 2 commits into
mainfrom
cleanup/heredoc-2
Dec 5, 2025
Merged

[TASK] Avoid heredoc in another test#1411
JakeQZ merged 2 commits into
mainfrom
cleanup/heredoc-2

Conversation

@oliverklee

Copy link
Copy Markdown
Collaborator

This allows the testcase being autoformatted
without breaking the tests for PHP 7.2.

https://www.php.net/manual/en/language.types.string.php#language.types.string.syntax.heredoc

@coveralls

coveralls commented Dec 4, 2025

Copy link
Copy Markdown

Coverage Status

coverage: 62.706%. remained the same
when pulling 551f210 on cleanup/heredoc-2
into dc99bbd on main.

@JakeQZ JakeQZ left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we also indent the contents of the string, etc., as part of this change? It would make the code easier to read.

I don't think the extra whitepsace should matter for the tests - the change as proposed may already be introducing or removing line-feeds (or maybe not - I'm not sure).

If it does matter, would it be possible to use string concatenation - or is that not an option for a constant in PHP 7.2?

This allows the testcase being autoformatted
without breaking the tests for PHP 7.2.

https://www.php.net/manual/en/language.types.string.php#language.types.string.syntax.heredoc
@oliverklee

Copy link
Copy Markdown
Collaborator Author

If I change the indentation, some tests will fail. I'll try concatenated strings next.

@JakeQZ JakeQZ left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Some of the tests are unnecessarily whitespace-dependent :/

@JakeQZ JakeQZ merged commit 00f637f into main Dec 5, 2025
23 checks passed
@JakeQZ JakeQZ deleted the cleanup/heredoc-2 branch December 5, 2025 11:28
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