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.

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

APIv4 - Rename 'ReadOnly' trait to 'ReadOnlyEntity' to avoid PHP error #22626

Merged
merged 1 commit into from
Jan 27, 2022
Merged

APIv4 - Rename 'ReadOnly' trait to 'ReadOnlyEntity' to avoid PHP error #22626

merged 1 commit into from
Jan 27, 2022

Conversation

colemanw
Copy link
Member

Overview

Fixes dev/core#3043

Before

Crashes PHP 8.1+

After

No crash

Technical Details

The word 'readonly' is reserved as of php 8.1

@civibot
Copy link

civibot bot commented Jan 26, 2022

(Standard links)

@civibot civibot bot added the 5.46 label Jan 26, 2022
@eileenmcnaughton
Copy link
Contributor

@colemanw did it also hate ReadOnlyEntity? (feels like a half a camel with ReadonlyEntity

@ejegg

@ejegg
Copy link
Contributor

ejegg commented Jan 26, 2022 •
edited
Loading

agreed, FullCamel would be better than HalfaCamel :)
Thanks for the quick patch!

@colemanw
Copy link
Member Author

Ok sure, I can change it. The reason I did ReadonlyEntity is because readonly is one word in html. But most people probably consider it two :)

@colemanw colemanw changed the title APIv4 - Rename 'ReadOnly' trait to 'ReadonlyEntity' to avoid PHP error APIv4 - Rename 'ReadOnly' trait to 'ReadOnlyEntity' to avoid PHP error Jan 26, 2022
@colemanw
APIv4 - Rename 'ReadOnly' trait to 'ReadOnlyEntity' to avoid PHP fata…
405d086 
…l error

Fixes dev/core#3043
The word 'readonly' is reserved as of php 8.1
@eileenmcnaughton
Copy link
Contributor

@colemanw this seems mergeable - is this the full fix?

@eileenmcnaughton eileenmcnaughton merged commit 01964ce into civicrm:5.46 Jan 27, 2022
@eileenmcnaughton eileenmcnaughton deleted the readOnlyRename branch January 27, 2022 00:59
@colemanw
Copy link
Member Author

Yep, that's it.

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
5.46
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@colemanw @eileenmcnaughton @ejegg