8000 [Form] Form view parent by bhavin-nakrani · Pull Request #19491 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Form] Form view parent #19491

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
Closed
wants to merge 8 commits into from
Closed

[Form] Form view parent #19491

wants to merge 8 commits into from

Conversation

bhavin-nakrani
Copy link
Contributor
Q A
Branch? "master" for new features
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #18882
License MIT
Doc PR reference to the documentation PR, if any

@bhavin-nakrani bhavin-nakrani changed the title Form view parent [Form] Form view parent Jul 30, 2016
HeahDude
HeahDude reviewed Jul 30, 2016
View reviewed changes
src/Symfony/Component/Form/FormView.php
*/
public function hasParent()
{
return (null !== $this->parent) ? true : false;
8000 Copy link
Contributor

Choose a reason for hiding this comment

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

The ternary is useless, just return null !== $this->parent

@HeahDude
Copy link
Contributor

The cs fixes in this PR are either needless or out of the scope here.

Status: Needs Work

@HeahDude
Copy link
Contributor
HeahDude commented Jul 30, 2016 •
edited
Loading

On second thought this may be a BC break: testing form.parent will return true if the view has a parent when one would expect to test if it has a child named "parent".

So I'm afraid we cannot merge this PR.

Ok this in fact don't change anything to the current testing, sorry for the confusion.

@bhavin-nakrani bhavin-nakrani deleted the form-view-parent branch July 30, 2016 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Feature Status: Needs Work
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bhavin-nakrani @HeahDude @carsonbot
0