Skip to content

[TypeInfo] Add space after glue comma #61430

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 19, 2025
Merged

Conversation

ruudk
Copy link
Contributor

@ruudk ruudk commented Aug 15, 2025

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

With this change, it will print a collection type as array<key, value> instead of array<key,value>.

I believe it is more standard than without the space. It's also what Symfony itself does everywhere.

Or should this target 8.0?

@carsonbot carsonbot added this to the 7.4 milestone Aug 15, 2025
@ruudk ruudk force-pushed the 7.4-glue branch 3 times, most recently from 3851224 to d447afb Compare August 15, 2025 09:17
@OskarStark
Copy link
Contributor

7.4 is fine

@ruudk
Copy link
Contributor Author

ruudk commented Aug 15, 2025

I'm not sure what's up with these tests. Every time I fix it, and run it, it keeps on failing.

Could it be that it's pulling in the officially tagged TypeInfo?

@nicolas-grekas nicolas-grekas modified the milestones: 7.4, 7.3 Aug 19, 2025
With this change, it will print a collection type as `array<key, value>` instead of
`array<key,value>`.

I believe it is more standard than without the space. It's also what Symfony itself does
everywhere.
@nicolas-grekas
Copy link
Member

Thank you @ruudk.

@nicolas-grekas nicolas-grekas merged commit 9e38523 into symfony:7.3 Aug 19, 2025
4 of 11 checks passed
fabpot added a commit that referenced this pull request Aug 19, 2025
This PR was merged into the 7.3 branch.

Discussion
----------

[JsonStreamer] fix tests

| Q             | A
| ------------- | ---
| Branch?       | 7.3
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        | following #61430
| License       | MIT

Commits
-------

3f25922 fix tests
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.

4 participants