Skip to content

56754 #3961

New issue

Have a question about this project? Sign up for a free account to open an issue and contact its maintainers and the community.

By clicking “Sign up for ”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on ? Sign in to your account

Closed
wants to merge 14 commits into from
Closed

56754 #3961

wants to merge 14 commits into from

Conversation

robinwpdeveloper
Copy link

Trac ticket: https://core.trac.wordpress.org/ticket/56754


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See Pull Requests for Code Review in the Core Handbook for more details.

Copy link
Contributor

@costdev costdev left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @robinwpdeveloper!

I've left a note inline that we can't change the case of a function parameter. This means that the changes in src/wp-admin/includes/taxonomy.php (Lines 189-196) can be made here. Can you update the PR to remove all other changes? 🙂

While this PR is the same as one of the es on the ticket, creating this PR helped to verify that no automated tests were affected by the change, so thanks for that! 👍

@robinwpdeveloper
Copy link
Author

Thanks @costdev
Changes are made as per feedback.

Copy link
Contributor

@costdev costdev left a comment

Choose a reason for hiding this comment

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

Thanks for the updates @robinwpdeveloper! LGTM 👍

@SergeyBiryukov
Copy link
Member

Thanks for the PR! Merged in r55190.

Sign up for free to join this conversation on . Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants