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

[PropertyAccess] Add nullsafe operator support#34497

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

@oliverde8
Copy link

@oliverde8oliverde8 commentedNov 21, 2019•
edited
Loading

QA
Branch?master
Bug fix?no
New feature?Yes
Deprecations?no
TicketsFix#34483
LicenseMIT
Doc PRsymfony/symfony-docs#...

TODO:

  • For arrays as well
  • Update unit test
  • Update documentation
  • Other this is my first MR?

@oliverde8oliverde8 changed the title[WIP] Feature 34483 - Having optional property accesses.[WIP][PropertyAccess] Feature 34483 - Having optional property accesses.Nov 21, 2019
@oliverde8oliverde8force-pushed thefeature-34483-optional-property-access branch from908618f to66f5307CompareNovember 21, 2019 17:23
@ogizanagiogizanagi added this to thenext milestoneNov 21, 2019
@oliverde8oliverde8force-pushed thefeature-34483-optional-property-access branch 2 times, most recently fromb22389d tobd0de68CompareNovember 24, 2019 11:34
@oliverde8
Copy link
Author

oliverde8 commentedNov 24, 2019•
edited
Loading

I have updated the the Pull Requests; the unit tests are updated as well.

I can update the documentation but before that I would like to know if the feature will be kept.

Regards,

Edit: There are errors with the ci's but other Pull Request seems to have the same issue with "already" existing table errors.

@oliverde8oliverde8force-pushed thefeature-34483-optional-property-access branch frombd0de68 to30a80a3CompareNovember 27, 2019 09:57
@oliverde8oliverde8force-pushed thefeature-34483-optional-property-access branch from30a80a3 to79b2fa1CompareNovember 27, 2019 10:00
@oliverde8oliverde8 changed the title[WIP][PropertyAccess] Feature 34483 - Having optional property accesses.[PropertyAccess] Feature 34483 - Having optional property accesses.Nov 27, 2019
Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

maybe "nullable" is a better wording for this? or "nullsafe"?
I had to open the RFC to get what "optional" was about.

*
* @throws Exception\OutOfBoundsException If the offset is invalid
*/
publicfunctionisOptional(int$index);

Choose a reason for hiding this comment

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

Missing return type.
Note that adding a new method on an existing interface is a BC break.
We use@method annotations in similar cases. See branch 4.4 for examples.

fabpot, ogizanagi, and yceruto reacted with thumbs up emoji
Copy link
Author

@oliverde8oliverde8Sep 16, 2020•
edited
Loading

Choose a reason for hiding this comment

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

Did it,

just when I am using it shouldn't I trigger a deprecation if the method is not set?
I found this example in the 4.4 branch and it's not present:https://github.com/symfony/symfony/blob/4.4/src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php#L120

@fabpot
Copy link
Member

@oliverde8 Still interested in finishing this PR?

@oliverde8
Copy link
Author

I missed nicolas-grekas last comment, I will do the changes this weekend.

sorry about that.

@oliverde8oliverde8force-pushed thefeature-34483-optional-property-access branch 2 times, most recently from5236eb4 to759432cCompareSeptember 16, 2020 11:55
@oliverde8oliverde8force-pushed thefeature-34483-optional-property-access branch from759432c to4996cd3CompareSeptember 16, 2020 12:00
…nal-property-access# Conflicts:#src/Symfony/Component/PropertyAccess/PropertyAccessor.php
@oliverde8oliverde8force-pushed thefeature-34483-optional-property-access branch from8ec7c15 tof1630f0CompareSeptember 16, 2020 12:18
if (method_exists($propertyPath,'isNullSafe')) {
// To be removed in symfony 6 once we are sure isNullSafe is always implemented.
$isNullSafe =$propertyPath->isNullSafe($i);
}
Copy link
Member

Choose a reason for hiding this comment

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

I think we need to add anelse clause here to raise a deprecation notice to help people with upgrading their code.

@fabpot
Copy link
Member

We've just moved away frommaster as the main branch to use5.x instead. Unfortunately, I cannot reopen the PR and change the target branch to5.x. Can you open a new PR referencing this one to not loose the discussion? Thank you for your understanding and for your help.

@ogizanagiogizanagi changed the title[PropertyAccess] Feature 34483 - Having optional property accesses.[PropertyAccess] Add nullsafe operator supportMay 24, 2022
fabpot added a commit that referenced this pull requestOct 1, 2022
…s (fsoedjede)This PR was merged into the 6.2 branch.Discussion----------[Form][PropertyAccess] Allow optional property accesses| Q             | A| ------------- | ---| Branch?       | 6.2| Bug fix?      | no| New feature?  | yes <!-- please update src/**/CHANGELOG.md files -->| Deprecations? | yes <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tickets       |Fix#34483 <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->| License       | MIT| Doc PR        |symfony/symfony-docs#17288 <!-- required for new features --><!--Replace this notice by a short README for your feature/bugfix.This will help reviewers and should be a good start for the documentation.Additionally (seehttps://symfony.com/releases): - Always add tests and ensure they pass. - Bug fixes must be submitted against the lowest maintained branch where they apply   (lowest branches are regularly merged to upper ones so they get the fixes too). - Features and deprecations must be submitted against the latest branch. - For new features, provide some code snippets to help understand usage. - Changelog entry should followhttps://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry - Never break backward compatibility (seehttps://symfony.com/bc).-->This PR is basically a copy of#34497 with some minor changes.I needed it and I have some time now to help advance in implementing it.**TODO**- [x] Add nullsafe operator usage- [x] Update unit test- [x] Update documentationRegardsCommits-------6455feb feature#34483 - Allow optional property accesses
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

@fabpotfabpotfabpot requested changes

@xabbuhxabbuhAwaiting requested review from xabbuh

+1 more reviewer

@ogizanagiogizanagiogizanagi left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

5.4

Development

Successfully merging this pull request may close these issues.

[PropertyAccess] Add nullsafe operator support

5 participants

@oliverde8@fabpot@nicolas-grekas@ogizanagi@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp