Skip to content

rename 'Object' classes to 'OLEObject' (php 7.2 compatibility) #1185

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 65 commits into from
Dec 29, 2017

Conversation

SailorMax
Copy link
Contributor

If name is not very good, now easier to find all require places to rename ;)

@SailorMax
Copy link
Contributor Author

Scrutinizer said "Failed", but I can't see what's the problem?

@SailorMax SailorMax force-pushed the php72_support_object_classes branch from 6a7a180 to 2a84625 Compare November 11, 2017 15:20
@troosan troosan mentioned this pull request Dec 4, 2017
4 tasks
@troosan
Copy link
Contributor

troosan commented Dec 5, 2017

@SailorMax not a big fan of the name ObjectElement, it is too different from all other elements.
What about OLEObject?

@SailorMax
Copy link
Contributor Author

I think you better know.
I don't know internal structure of PHPWord and used ObjectElement as name similar to AbstractElement.
If it looks like OLEObject, you can rename it :)

@troosan troosan added this to the v0.15.0 milestone Dec 20, 2017
@troosan troosan merged commit 400a8e6 into PHPOffice:develop Dec 29, 2017
@troosan
Copy link
Contributor

troosan commented Dec 29, 2017

still need tecnickcom/TCPDF#74 to be fully PHP 7.2 compatible

@troosan troosan changed the title rename 'Object' classes to 'ObjectElement' (php 7.2 compatibility) rename 'Object' classes to 'OLEObject' (php 7.2 compatibility) Dec 29, 2017
@stevechilds
Copy link

stevechilds commented May 13, 2018

// Ignore me, just realised the error wasn't in your code, but ours! It was in an include line.

@SailorMax SailorMax deleted the php72_support_object_classes branch May 24, 2018 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

10 participants