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

[php8-compat] [REF] Fix a couple of functions triggering deprecation notices in php… #20442

Conversation

seamuslee001
Copy link
Contributor

…8.0 where by an optional parameter is before required parameters in the function signature

Overview

This fixes a couple of deprecation notices found when installing dmaster on a local php8.0 instance

PHP Deprecated: Required parameter $fieldId follows optional parameter $fieldIdName in /home/seamus/buildkit/build/dmaster/web/sites/all/modules/civicrm/CRM/Core/DAO.php on line 2014

Deprecated: Required parameter $domainSpecific follows optional parameter $fieldName in /home/seamus/buildkit/build/dmaster/web/sites/all/modules/civicrm/CRM/Core/OptionValue.php on line 254

Before

Deprecation notices emitted in php8.0

After

No deprecation notices emitted for these functions in php8.0

ping @eileenmcnaughton @totten @colemanw

@civibot
Copy link

civibot bot commented May 29, 2021

(Standard links)

@civibot civibot bot added the master label May 29, 2021
@eileenmcnaughton
Copy link
Contributor

@seamuslee001 perhaps we should go the other way - & make them non-optional. That is basically the status quo because calls that did not pass them in currently would fail

@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton hmm maybe I guess so just remove the defaults then?

@eileenmcnaughton eileenmcnaughton changed the title [REF] Fix a couple of functions triggering deprecation notices in php… [php8-compat] [REF] Fix a couple of functions triggering deprecation notices in php… May 29, 2021
@eileenmcnaughton
Copy link
Contributor

Yeah I think so

…8.0 where by an optional parameter is before required parameters in the function signature
@seamuslee001 seamuslee001 force-pushed the fix_required_param_after_optional branch from e231bd1 to b05a747 Compare May 29, 2021 03:25
@seamuslee001
Copy link
Contributor Author

ok have updated to that now @eileenmcnaughton

@eileenmcnaughton eileenmcnaughton merged commit 9a6ee01 into civicrm:master May 29, 2021
@eileenmcnaughton eileenmcnaughton deleted the fix_required_param_after_optional branch May 29, 2021 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants