Skip to content

[Component][Serializer][Normalizer] : Deal it with Has Method for the Normalizer/Denormalizer #23337

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 6, 2017

Conversation

jsamouh
Copy link
Contributor

@jsamouh jsamouh commented Jun 30, 2017

Q A
Branch? 3.4
Bug fix? no
New feature? yes
BC breaks? ?
Deprecations? no
Tests pass? yes
Fixed tickets #23314
License MIT
Doc PR symfony/symfony-docs

Deal it with Has Method for the Normalizer/Denormalizer

@jsamouh
Copy link
Contributor Author

jsamouh commented Jun 30, 2017

@Aliance , I stole you this mini practice to exercise my Symfony PR ;-) (I'm a newbie)
I will be very interresting if you can comment this PR
Note: For unit test, I did not want to change the existing unit test to add has test method. I prefered to create new unit test with new class

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Jul 3, 2017
@nicolas-grekas nicolas-grekas requested a review from dunglas July 3, 2017 07:57
Copy link
Member

@dunglas dunglas left a comment

Choose a reason for hiding this comment

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

Thanks for this fix! I left some minor comments.

@@ -497,6 +497,23 @@ public function testPrivateSetter()
$this->assertEquals('bar', $obj->getFoo());
}

public function testHasGetterDenormalize()
{
$obj = $this->normalizer->denormalize(array('foo' => true), __NAMESPACE__.'\ObjectWithHasGetterDummy');
Copy link
Member

Choose a reason for hiding this comment

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

You can use ObjectWithHasGetterDummy::class

public function testHasGetterDenormalize()
{
$obj = $this->normalizer->denormalize(array('foo' => true), __NAMESPACE__.'\ObjectWithHasGetterDummy');
$this->assertEquals(true, $obj->hasFoo());
Copy link
Member

Choose a reason for hiding this comment

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

$this->assertTrue($obj->hasFoo())


class ObjectWithHasGetterDummy
{
private $foo = null;
Copy link
Member

Choose a reason for hiding this comment

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

= null is useless, it's the default value.

@jsamouh
Copy link
Contributor Author

jsamouh commented Jul 5, 2017

@dunglas , you can check :-)

Copy link
Contributor

@Aliance Aliance left a comment

Choose a reason for hiding this comment

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

Rebase should be done right before merging, not while code review. Otherwise reviewers can not see only made changes.

@fabpot
Copy link
Member

fabpot commented Jul 6, 2017

Thank you @jordscream.

@fabpot fabpot merged commit a15829d into symfony:3.4 Jul 6, 2017
fabpot added a commit that referenced this pull request Jul 6, 2017
… Method for the Normalizer/Denormalizer (jordscream)

This PR was merged into the 3.4 branch.

Discussion
----------

[Component][Serializer][Normalizer] : Deal it with Has Method for the Normalizer/Denormalizer

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | ?
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #23314
| License       | MIT
| Doc PR        | symfony/symfony-docs

Deal it with Has Method for the Normalizer/Denormalizer

Commits
-------

a15829d [Component][Serializer][Normalizer] : Deal it with Has Method for the Normalizer/Denormalizer
This was referenced Oct 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants