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

Narrow existing return types on private/internal/final/test methods#42213

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

Merged
nicolas-grekas merged 1 commit intosymfony:6.0fromnicolas-grekas:more-ret-types
Aug 10, 2021

Conversation

@nicolas-grekas
Copy link
Member

QA
Branch?6.0
Bug fix?no
New feature?no
Deprecations?no
Tickets-
LicenseMIT
Doc PR-

More progress from#42149

@nicolas-grekas
Copy link
MemberAuthor

(psalm issues are false positives, with a slight code improvement inef84e66)

@nicolas-grekasnicolas-grekasforce-pushed themore-ret-types branch 2 times, most recently fromf08a602 to97e8f39CompareJuly 22, 2021 11:50
@nicolas-grekas
Copy link
MemberAuthor

PR is ready @symfony/mergers

@nicolas-grekasnicolas-grekasforce-pushed themore-ret-types branch 3 times, most recently frome302e94 tob2de6c3CompareAugust 9, 2021 12:00
@nicolas-grekas
Copy link
MemberAuthor

PR rebased and ready.

Copy link
Member

@chalasrchalasr left a comment

Choose a reason for hiding this comment

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

LGTM. I need to take a nap now 😅

derrabus and nicolas-grekas reacted with laugh emoji
@nicolas-grekasnicolas-grekas merged commit061bf35 intosymfony:6.0Aug 10, 2021
@nicolas-grekasnicolas-grekas deleted the more-ret-types branchAugust 10, 2021 10:17
nicolas-grekas added a commit that referenced this pull requestAug 25, 2021
This PR was merged into the 6.0 branch.Discussion----------Add back ``@return` $this` annotations| Q             | A| ------------- | ---| Branch?       | 6.0| Bug fix?      | no| New feature?  | no| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -In#42213 I removed those annotations and tried to convinced myself and others that we could do so without loosing too much.I was about to send a PR to remove all remaining similar annotations. When I reviewed the patch I created for that, I stopped on `ItemInterface::tag()` (which is just an example):https://github.com/symfony/symfony/blob/444d43fa11990092c53ba54849bb1df36225adcf/src/Symfony/Contracts/Cache/ItemInterface.php#L52-L57If we remove that annotation on the interface, all implementations will have to deal with the return value of the call to `tag()`. Whereas with ``@return` $this`, the contracts are clear: no need to, implementations are expected to mutate and return the current object. I don't think this would be appropriate in this example./cc `@nikic` as this might be some nice food for thoughts to consider adding `$this` as a possible native return type.Commits-------7fafe83 Add back ``@return` $this` annotations
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@derrabusderrabusderrabus approved these changes

@chalasrchalasrchalasr approved these changes

@dunglasdunglasAwaiting requested review from dunglas

@jderussejderusseAwaiting requested review from jderussejderusse is a code owner

@OskarStarkOskarStarkAwaiting requested review from OskarStarkOskarStark is a code owner

@srozesrozeAwaiting requested review from sroze

@ycerutoycerutoAwaiting requested review from ycerutoyceruto is a code owner

@xabbuhxabbuhAwaiting requested review from xabbuh

@wouterjwouterjAwaiting requested review from wouterjwouterj is a code owner

+1 more reviewer

@TobionTobionTobion left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

6.0

Development

Successfully merging this pull request may close these issues.

5 participants

@nicolas-grekas@Tobion@derrabus@chalasr@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp