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: updates function parameter #5277

Closed
wants to merge 1 commit into from

Conversation

kebbet
Copy link

@kebbet kebbet commented Sep 22, 2023

@mukeshpanchal27
Copy link
Member

@kebbet, do we have any documentation regarding or any guidelines for this?

@kebbet
Copy link
Author

kebbet commented Sep 22, 2023

@kebbet, do we have any documentation regarding or any guidelines for this?

Not really atm, just looking at previous efforts, like https://core.trac.wordpress.org/changeset/54956 which was commited to his ticket https://core.trac.wordpress.org/ticket/56788

@mukeshpanchal27
Copy link
Member

Thanks That changes was committed by @SergeyBiryukov so let's ping him for the clarification. Also i ping @costdev if hi knows anything about for it.

Copy link
Member

@mukeshpanchal27 mukeshpanchal27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look fine to me.

@kebbet
Copy link
Author

kebbet commented Sep 22, 2023

This is the ticket för 6.4 -> https://core.trac.wordpress.org/ticket/58976

@joedolson
Copy link
Contributor

I think that @SergeyBiryukov's commit message is pretty clear here; we're avoiding using PHP keywords in variable names. So I don't think we need any additional confirmation; this is good to go.

@joedolson
Copy link
Contributor

In r56662

@joedolson joedolson closed this Sep 22, 2023
@kebbet kebbet deleted the 57791-named-parameter branch September 22, 2023 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants