Opened 6 years ago
Closed 5 years ago
#48246 closed defect (bug) (wontfix)
Explicitly state priority in remove_filter on `remove_filter( 'upgrader_post_install', array( $this, 'check_parent_theme_filter' ) )`
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 5.3 |
Component: | Themes | Keywords: | has-patch close 2nd-opinion |
Focuses: | coding-standards | Cc: |
Description
Just for consistency we can add the priority in the remove_filter
.
Attachments (1)
Change History (7)
#4
@
5 years ago
- Keywords close 2nd-opinion added
Sorry, can't say I see the point of this?
A priority is only added in the remove_filter
if the original add_filter
had a priority other than 10. When it uses the default there's no benefit. Throughout core I've only seen non-default priorities with the exception of some unit tests.
Apologies if I'm completely overlooking something. If we do set this as a precedent would all remove_filter implementations in core then need to be updated to add the priority?
#5
@
5 years ago
- Keywords commit removed
@garrett-eclipse you make a good point. The patch is simple enough but I did not check if others throughout core were removed with a priority as well. If your saying it's not done with default priorities - only with others that have non-default priorities then can close this one out.
I don't see making one adjustment though as setting a president for all actions in core to the point where they would all need updated. Just one here wouldn't affect anything else.
It's one and the same - makes no difference if this is added here or not. Let's leave this one open and see if others see something else that we don't but I'll remove the commit tag and keep the 'close' on your recommending :)
#6
@
5 years ago
- Milestone 5.4 deleted
- Resolution set to wontfix
- Status changed from new to closed
Thanks for the patch!
I agree with comment:4. Looking at Theme_Upgrader::install() the priority is only passed to the add_filter()
call so that the number of arguments (3) could be passed next.
Since 10 is the default priority, adding it to the remove_filter()
call doesn't seem to provide any benefit or affect consistency here.
Patch is good, logic for why it could be added here makes sense to me - consistency with other removes.
Nothing blocking this one unless other have alternative opinions. It's ready for commit.