-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Add $context
parameter to get_edit_comment_link()
to get the URL without HTML entities
#7071
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 $context
parameter to get_edit_comment_link()
to get the URL without HTML entities
#7071
Conversation
Add `$context` parameter to `get_edit_comment_link()` to get the URL without HTML entities
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @deepakro. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. Core Committers: Use this line as a base for the props when committing in SVN:
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
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.
@deepakrohillas Thank you for the pull request!
There are a few quirks to iron out on this one, per my feedback below.
Would you also be open to implementing a unit test for the new $context
parameter? You could create a new file tests/phpunit/tests/link/getEditCommentLink.php
with a corresponding test class for it, that tests the use of the $context
parameter.
As a starting point for the test class, you could review the getEditTermLink.php
file which is for a similar kind of function. Of course, no need to cover the entire function with tests, only covering the $context
parameter is enough, since that's what this PR is about.
Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>
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.
@deepakrohillas Thank you for the updates, this looks good to me production code-wise. Just one more comment regarding the documentation.
Did you see my note about adding tests for this in #7071 (review)? Please let me know if you're interested in doing that or if you prefer for someone else to take care of that.
Co-authored-by: Felix Arntz <flixos90@gmail.com>
@felixarntz Thanks for the suggestion , i will update you today EOD for test case. Thanks again!. |
@felixarntz : test case also added please review. |
Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>
@mukeshpanchal27 : Suggested changes performed , please check Thanks!. |
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.
@deepakrohillas Apologies for the delay in review, and thank you for adding all the tests!
Looks good for the most part. A few more notes to polish it before we can commit it to WordPress core.
@felixarntz Please check it now |
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.
@deepakrohillas Pretty much good to go, now there's only a little bit of clean-up to do. I'm happy to quickly apply those changes prior to commit.
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.
@deepakrohillas Thank you for continuing to work on this PR!
Implemented the final polishing, should be good to commit now :)
Committed in https://core.trac.wordpress.org/changeset/58875 |
Trac ticket: https://core.trac.wordpress.org/ticket/61727