#43041 closed enhancement (fixed)
Rename the capital_P_dangit function
Reported by: | danieltj | Owned by: | jrf |
---|---|---|---|
Milestone: | 5.1 | Priority: | normal |
Severity: | normal | Version: | |
Component: | General | Keywords: | has-patch |
Focuses: | coding-standards | Cc: |
Description (last modified by )
I'm not too bothered about the capital_P_dangit function in terms of what it does. Correcting WordPress to WordPress is fine by me. Capitalise the P
and it never worries you.
My problem with this function is the name and documentation of it. Taken from a 4.9.1 install:
wp-includes/formatting.php, line 4805:
Forever eliminate "Wordpress" from the planet (or at least the little bit we can influence). Violating our coding standards for a good function name.
As funny as the name might seem the first time around, violating the coding standards doesn't seem like something that should live on in core for the sake of a gag. I'd like to propose that capital_P_dangit
is deprecated and used as a wrapper function for a newly named wp_brand_awareness
.
I personally think wp_brand_awareness
is a better name, fits in with coding standards and still does what it's set out to do, but in a more 'WordPress-esque' way. I'd also suggest updating the documentation to remove the comment about from our planet
and for a gag
lines too.
Attachments (1)
Change History (12)
#1
follow-up:
↓ 9
@
7 years ago
- Milestone changed from Awaiting Review to 5.0
- Owner set to pento
- Status changed from new to assigned
- Version 4.9.1 deleted
#4
@
7 years ago
@pento phpcs:disable/set
annotations are not file-specific, so always need to be followed by a phpcs:enable/set
command to reset to the original ruleset.
With phpcs:set
, this would be a set
annotation resetting the value to the default or the value as set by the WPCS ruleset.
I've attached a patch to fix this for the new unit test file.
#5
follow-up:
↓ 11
@
7 years ago
Huh, interesting. I tested for that, and it seemed to automatically be re-enabled in a subsequent file.
#7
@
7 years ago
- Keywords has-patch commit added
- Owner changed from pento to jrf
- Status changed from reopened to assigned
#8
@
7 years ago
Huh, interesting. I tested for that, and it seemed to automatically be re-enabled in a subsequent file.
Hmm.. something might have changed with the new annotations in that case. I'll have a further look.
I know for sure that the old style annotations are not file-specific as we've run into this exact issue when unit testing sniffs.
Also - the file order is not predetermined and can be different across systems, quite apart from caching and parallel processing influencing the results, so the end-results may be misleading.
All the same, resetting/re-enabling whenever these annotations are used is never a bad habit to get used to.
#9
in reply to:
↑ 1
@
7 years ago
Replying to pento:
Renaming it is definitely out, but I agree that the coding standards warnings are a problem. Let's see what I can do... 😉
How come?
The description even says:
Violating our coding standards for a good function name.
Surely the idea is to ensure everything is kept consistent across the code base? I'd insist that it's changed to be more appropriate as whilst it's amusing to begin with, violating code standards for a joke wouldn't be tolerated elsewhere.
#10
@
6 years ago
- Description modified (diff)
- Keywords commit removed
- Milestone changed from 5.0 to 5.1
- Resolution set to fixed
- Status changed from assigned to closed
I agree with Pento here. Renaming this function introduces backwards compatibility issues and is a breaking change. Deprecating it in favour of a better named function is more effort than it's worth.
Closing as fixed for 5.1 due to the sniffer comments introduced in [42429].
#11
in reply to:
↑ 5
@
6 years ago
Huh, interesting. I tested for that, and it seemed to automatically be re-enabled in a subsequent file.
Just wanted to confirm that. Per https://github.com/squizlabs/PHP_CodeSniffer/wiki/Advanced-Usage:
Note: All
phpcs:disable
andphpcs:enable
comments only apply to the file they are contained within. After the file has finished processing all sniffs are re-enabled for future files.
Renaming it is definitely out, but I agree that the coding standards warnings are a problem. Let's see what I can do... 😉