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

Fix for Exception #29

Merged
merged 3 commits into from
Nov 10, 2015
Merged

Fix for Exception #29

merged 3 commits into from
Nov 10, 2015

Conversation

Jan0707
Copy link
Contributor

@Jan0707 Jan0707 commented Nov 9, 2015

We cannot assume that EVERY object we receive here implements the getName method. Therefore we should fall back to the ReflectionObject which implements the method (via ReflectionClass).

We cannot assume that EVERY object we receive here implements the getName method. Therefore we should fall back to the ReflectionObject which implements the method (via ReflectionClass).
@jkuchar
Copy link
Contributor

jkuchar commented Nov 9, 2015

Good point! 👍

@mnapoli
Copy link
Member

mnapoli commented Nov 9, 2015

Oh good catch! That was probably a typo, could you please fix the test (https://travis-ci.org/myclabs/DeepCopy/jobs/90081673) so that the build is green?

@Jan0707
Copy link
Contributor Author

Jan0707 commented Nov 9, 2015

Kind of busy right now, but I can have a look into it over the next few days.

@jkuchar
Copy link
Contributor

jkuchar commented Nov 9, 2015

I will fix this, looks like a one-line fix.

@jkuchar
Copy link
Contributor

jkuchar commented Nov 9, 2015

Opened PR for this PR. :-)

@mnapoli
Copy link
Member

mnapoli commented Nov 9, 2015

👍 that's meta, will merge once it's merged ;)

Fixed wrong reference in Exception message
@Jan0707
Copy link
Contributor Author

Jan0707 commented Nov 10, 2015

Sorry for the delay :)

@jkuchar
Copy link
Contributor

jkuchar commented Nov 10, 2015

If you want to get rid of merge commit just do rebase. (but it is not technicaly neccesary)

@mnapoli
Copy link
Member

mnapoli commented Nov 10, 2015

Perfectly fine like this! Thanks both!

mnapoli added a commit that referenced this pull request Nov 10, 2015
@mnapoli mnapoli merged commit 8cbfb7b into myclabs:master Nov 10, 2015
@tchiotludo tchiotludo mentioned this pull request Apr 25, 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.

3 participants