Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[DependencyInjection] Add types to private/final/internal methods and constructors#33266

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

@derrabus
Copy link
Member

QA
Branch?4.4
Bug fix?no
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#32179,#33228
LicenseMIT
Doc PRNA/

@nicolas-grekasnicolas-grekas added this to thenext milestoneAug 21, 2019
nicolas-grekas added a commit that referenced this pull requestAug 21, 2019
…hods (nicolas-grekas, derrabus)This PR was merged into the 4.4 branch.Discussion----------Add return types to tests and final|internal|private methods| Q             | A| ------------- | ---| Branch?       | 4.4| Bug fix?      | no| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#33228#33236| License       | MIT| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->#33266 made me spot an issue with my logic to add return types with help from `DebugClassLoader`.Here is a completing PR, with the logic expanded to test classes.I need help to fix tests, /cc@derrabus :)Commits-------c39fd9c Fixed tests on the Security and Form componentsfc186bb Add return types to tests and final|internal|private methods
@nicolas-grekas
Copy link
Member

nicolas-grekas commentedAug 21, 2019•
edited
Loading

Honestly, I'm not a big fan of addingvoid. e.g. on test cases, they are pure visual noise.
Can you please rebase this PR, and maybe move the void return types to another PR if you want to push them forward?

@derrabus
Copy link
MemberAuthor

Honestly, I'm not a big fan of addingvoid. e.g. on test cases

Hey, I could have addedvoid to alltest* methods, but I thought that might hurt your eyes a little too much. 😅

As I already said on#33228, I might need a llittle guidance on when to addvoid. Sometimes you addvoid yourself, sometimes you state that you don't likevoid.

@derrabusderrabusforce-pushed theimprovement/dependency-injection-types branch fromc21fbb2 to767023fCompareAugust 21, 2019 16:10
@nicolas-grekas
Copy link
Member

nicolas-grekas commentedAug 21, 2019•
edited
Loading

Sometimes you add void yourself, sometimes you state that you don't like void.

:'D I did add them when a third-party dep encourages us to do so. I may have added more but I regret already :)

@derrabusderrabusforce-pushed theimprovement/dependency-injection-types branch from767023f tofb74fbfCompareAugust 30, 2019 16:50
@derrabus
Copy link
MemberAuthor

If you've heard someone weeping, it was probably me reverting all thevoid declarations. PR is ready now. 😃

sstok and nicolas-grekas reacted with laugh emoji

@derrabusderrabusforce-pushed theimprovement/dependency-injection-types branch fromfb74fbf todef0ac7CompareSeptember 8, 2019 19:38
@nicolas-grekas
Copy link
Member

Thank you@derrabus.

nicolas-grekas added a commit that referenced this pull requestSep 8, 2019
…l methods and constructors (derrabus)This PR was merged into the 4.4 branch.Discussion----------[DependencyInjection] Add types to private/final/internal methods and constructors| Q             | A| ------------- | ---| Branch?       | 4.4| Bug fix?      | no| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#32179,#33228| License       | MIT| Doc PR        | NA/Commits-------def0ac7 Add types to private/final/internal methods and constructors.
@nicolas-grekasnicolas-grekas merged commitdef0ac7 intosymfony:4.4Sep 8, 2019
@derrabusderrabus deleted the improvement/dependency-injection-types branchSeptember 8, 2019 21:36
@nicolas-grekasnicolas-grekas modified the milestones:next,4.4Oct 27, 2019
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@dunglasdunglasAwaiting requested review from dunglasdunglas is a code owner

1 more reviewer

@sstoksstoksstok left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

4 participants

@derrabus@nicolas-grekas@sstok@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp