Skip to content
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

Improve property access and added post processing #13

Merged
merged 12 commits into from
Dec 17, 2016
Merged

Improve property access and added post processing #13

merged 12 commits into from
Dec 17, 2016

Conversation

andreasschacht
Copy link
Contributor

Hi,
I made some improvements, e.g. access to private/protected properties. I got a request to make a PR ans ask you, if you can merge and create a new release. So others can also use this approvements and install them via packagist.

Best regards

"symfony/options-resolver": "~2.1",
"symfony/filesystem": "~2.1",
"phpcollection/phpcollection": "0.2.*",
"symfony/filesystem": "3.0.*",
Copy link
Owner

Choose a reason for hiding this comment

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

Please use "~2.1 | ~3.0"

"symfony/property-access": "~2.2",
"jms/cg": "1.0.0"
"jms/cg": ">=1.0.0"
Copy link
Owner

Choose a reason for hiding this comment

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

This would also allow version 2, 3, etc. So you should use ~1.0 instead

protected $skipNullValues = false;



Copy link
Owner

Choose a reason for hiding this comment

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

Remove two extra lines

* User: andreasschacht
* Date: 01.08.16
* Time: 11:27
*/
Copy link
Owner

Choose a reason for hiding this comment

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

Remove this comment block

* @return array
*/
public function getData($object, ClassMetaData $metadata, array $data);
}
Copy link
Owner

Choose a reason for hiding this comment

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

Please make sure that you always add a new line at the end of a file. Since you use PHPStorm, there is an option you can activate

image

* @return mixed
*/
public function getValue($object, $property);
}
Copy link
Owner

Choose a reason for hiding this comment

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

Same here

*
* @return boolean true, if this class can process the data
*/
public function isSupporting($object, ClassMetaData $metadata, array $data);
Copy link
Owner

Choose a reason for hiding this comment

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

Would call this method just supports

@Spea
Copy link
Owner

Spea commented Dec 16, 2016

Thx for this PR, but make sure that the tests are running again ;)

@andreasschacht
Copy link
Contributor Author

Thanks, I fixed it and renamed the method name. I had to restrict the jms/cg version to be less than v1.2, because this version breaks your tests. It adds a newline to the DocBlock of the generated class (with the ClassFixtureGenerator), the versions before don't. So your string comparison will fail.

@Spea Spea merged commit 1e66207 into Spea:master Dec 17, 2016
@Spea
Copy link
Owner

Spea commented Dec 17, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants