8000 [Components] Convert to native return types by wouterj · Pull Request #50852 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Components] Convert to native return types #50852

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 5, 2023
Merged

[Components] Convert to native return types #50852

merged 1 commit into from
Jul 5, 2023

Conversation

wouterj
Copy link
Member
@wouterj wouterj commented Jul 2, 2023 •
edited by nicolas-grekas
Loading
Q A
Branch? 7.0
Bug fix? no
New feature? no
Deprecations? no
Tickets -
License MIT
Doc PR -

@carsonbot carsonbot added this to the 7.0 milestone Jul 2, 2023
@nicolas-grekas
Copy link
Member

rebase unlocked + some fixes needed I guess

@nicolas-grekas nicolas-grekas changed the title [WIP] [Components] Convert to native return types [Components] Convert to native return types Jul 5, 2023
@nicolas-grekas nicolas-grekas force-pushed the convert-to-native-types-2 branch from e3220b5 to 824f48a Compare July 5, 2023 12:38
@nicolas-grekas nicolas-grekas mentioned this pull request Jul 5, 2023
Merged
nicolas-grekas added a commit that referenced this pull request Jul 5, 2023
@nicolas-grekas
minor #50889 [Security] Add native return type to `Firewall::getSubsc…
e869eb6 
…ribedEvents()` (nicolas-grekas)

This PR was merged into the 6.4 branch.

Discussion
----------

[Security] Add native return type to `Firewall::getSubscribedEvents()`

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Required to make #50852 green without breaking compat between v6 and v7.

Commits
-------

8069f49 [Security] Add native return type to `Firewall::getSubscribedEvents()`
@nicolas-grekas nicolas-grekas force-pushed the convert-to-native-types-2 branch from 824f48a to a250774 Compare July 5, 2023 13:06
nicolas-grekas
nicolas-grekas approved these changes Jul 5, 2023
View reviewed changes
Copy link
Member
@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

I finished the PR.

I order to help third party bundles to be compatible with Symfony 7, I didn't add the void return type to the following interfaces (all listed in .github/expected-missing-return-types.diff):

  • src/Symfony/Component/DependencyInjection/Compiler/CompilerPassInterface.php
  • src/Symfony/Component/DependencyInjection/Extension/PrependExtensionInterface.php
  • src/Symfony/Component/HttpKernel/Bundle/BundleInterface.php

Please open an issue or send a PR if you think we should add more interfaces to this list.

Note that src/Symfony/Contracts/Service/ResetInterface.php also doesn't have a native type, and that's on purpose, to keep compatibility with implementations that do have a return value.

I also patched DebugClassLoader so that it doesn't skip void return types.

@nicolas-grekas
Copy link
Member

Thank you @wouterj.

@nicolas-grekas nicolas-grekas merged commit a6ec131 into symfony:7.0 Jul 5, 2023
@wouterj wouterj deleted the convert-to-native-types-2 branch July 5, 2023 13:51
@wouterj wouterj mentioned this pull request Jul 17, 2023
Closed
@fabpot fabpot mentioned this pull request Oct 21, 2023
Merged
@mbabker mbabker mentioned this pull request Jan 31, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

@lyrixx lyrixx Awaiting requested review from lyrixx lyrixx is a code owner

@chalasr chalasr Awaiting requested review from chalasr chalasr is a code owner

@dunglas dunglas Awaiting requested review from dunglas dunglas is a code owner

@yceruto yceruto Awaiting requested review from yceruto yceruto is a code owner

@jderusse jderusse Awaiting requested review from jderusse jderusse is a code owner

@xabbuh xabbuh Awaiting requested review from xabbuh xabbuh is a code owner

@OskarStark OskarStark Awaiting requested review from OskarStark OskarStark is a code owner

Assignees
No one assigned
Labels
Status: Reviewed
Projects
None yet
Milestone
7.0
Development

Successfully merging this pull request may close these issues.
3 participants
@wouterj @nicolas-grekas @carsonbot
0