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

Re-order function parameters due to PHP8 deprecation notice #3043

Merged
merged 2 commits into from
Jul 27, 2022
Merged

Re-order function parameters due to PHP8 deprecation notice #3043

merged 2 commits into from
Jul 27, 2022

Conversation

g-a-c
Copy link
Contributor

@g-a-c g-a-c commented Jul 15, 2022

This changes the parameters for four functions in HAProxy.php.

The ACME Client appears to call these functions with positional rather than named params so also switch the order of the parameters in these function calls

Fixes: #3042

This changes the parameters for four functions in HAProxy.php.

The ACME Client appears to call these functions with positional rather than named params
so also switch the order of the parameters in these function calls in
@g-a-c g-a-c marked this pull request as ready for review July 15, 2022 20:50
@ichnograph
Copy link

This seems to be a very straightforward fix, and it fixes the RC for me. Without it, the ACME client is crashing. What is stopping it from getting merged?

@fraenki
Copy link
Member

fraenki commented Jul 26, 2022

@jvthert Time constraints and other priorities. :)

So you've tested this on an actual OPNsense installation and you can confirm that it's fully functional, right?

@fraenki fraenki added the bug Production bug label Jul 26, 2022
@fichtner
Copy link
Member

As far as I can see this is cosmetic and deprecation notes shouldn't even be issued on production setting, but I could be wrong...

@fichtner fichtner added cleanup Low impact changes and removed bug Production bug labels Jul 26, 2022
@fichtner
Copy link
Member

So despite excluding E_DEPRECATED it seems to emit those for "reasons".

https://github.com/opnsense/core/blob/1fa0cb3e2d18fd68cfeead29a72f033436d284ec/src/opnsense/service/templates/OPNsense/WebGui/php.ini#L24

FWIW, 22.7 is already built and in image testing phase so this will likely be included in 22.7.1.

@fichtner
Copy link
Member

Ok, for one caused by migration script, but also indicating it's benign https://github.com/opnsense/core/blob/1fa0cb3e2d18fd68cfeead29a72f033436d284ec/src/opnsense/mvc/script/run_migrations.php#L31

@fraenki fraenki merged commit 7d682dd into opnsense:master Jul 27, 2022
@fraenki
Copy link
Member

fraenki commented Jul 27, 2022

Merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Low impact changes
Development

Successfully merging this pull request may close these issues.

net/haproxy: PHP deprecation warnings
4 participants