-
Notifications
You must be signed in to change notification settings - Fork 293
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
add function and replace logic #5084
add function and replace logic #5084
Conversation
@felixarntz I think I'm struggling with the concepts here, but I made some change there. Can you help me review if the code I wrote matches to your guide. Thank you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sancodes Thanks for the PR! At a high level the logic looks right, but there are a couple things that need to be fixed.
Most importantly, check for valid PHP and following the WordPress PHP coding standards.
@felixarntz Sorry if the issue that I've fixed is still wrong. But I read your suggestions and tried recreating it. Can you help me review. I'm willing to fix if there's any issue. Thank you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sancodes Left a few remaining suggestions.
@felixarntz Is it looking good now? If not I'm willing to fix the mistakes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two more things need to be fixed here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sancodes I've made the last few tweaks here, this is good to go now. Thanks for your contribution!
Summary
Addresses issue:
Relevant technical choices
PR Author Checklist
Do not alter or remove anything below. The following sections will be managed by moderators only.
Code Reviewer Checklist
Merge Reviewer Checklist