TAXONOMY_NAME to PODCASTING_TAXONOMY_NAME#238
Conversation
|
New constant name is |
peterwilsoncc
left a comment
There was a problem hiding this comment.
LGTM but just one note inline. I've asked Darin for a logic check so it can wait until he gets back to us before making the change.
| define( 'TAXONOMY_NAME', 'podcasting_podcasts' ); | ||
| define( 'PODCASTING_TAXONOMY_NAME', 'podcasting_podcasts' ); | ||
| define( 'PODCASTING_ITEMS_PER_PAGE', 250 ); | ||
|
|
||
| trigger_error( 'The constant TAXONOMY_NAME is deprecated and will be removed in future versions. Use PODCASTING_TAXONOMY_NAME instead.', E_USER_DEPRECATED ); | ||
|
|
There was a problem hiding this comment.
The current set up of the plugin doesn't allow site owners to define the constant elsewhere to preempt the value (ie, there is no if ( defined( ... ) ) check). As such I think we can safely hard deprecate the value and only throw a notice if the old value is defined.
cc @dkotter for a logic check.
| define( 'TAXONOMY_NAME', 'podcasting_podcasts' ); | |
| define( 'PODCASTING_TAXONOMY_NAME', 'podcasting_podcasts' ); | |
| define( 'PODCASTING_ITEMS_PER_PAGE', 250 ); | |
| trigger_error( 'The constant TAXONOMY_NAME is deprecated and will be removed in future versions. Use PODCASTING_TAXONOMY_NAME instead.', E_USER_DEPRECATED ); | |
| define( 'PODCASTING_TAXONOMY_NAME', 'podcasting_podcasts' ); | |
| define( 'PODCASTING_ITEMS_PER_PAGE', 250 ); | |
| if ( defined( 'TAXONOMY_NAME' ) { | |
| $message = sprintf( | |
| /* translators: 1: TAXONOMY_NAME, 2: PODCASTING_TAXONOMY_NAME. */ | |
| __( 'The %1$s constant is no longer supported. Use %2$s instead.', 'simple-podcasting' ), | |
| 'TAXONOMY_NAME', | |
| 'PODCASTING_TAXONOMY_NAME' | |
| ); | |
| trigger_error( wp_strip_all_tags( $message ), E_USER_DEPRECATED ); | |
| } |
There was a problem hiding this comment.
This looks good to me, though as mentioned, we didn't really support this value being overridden in the first place so not sure there's a scenario where this deprecation notice would even be used. Doesn't hurt to have it in place though, so I'm fine with this change
There was a problem hiding this comment.
@peterwilsoncc
TAXONOMY_NAME has no prefix specific to Simple Podcasting plugin. What if any other script outside Simple Podcasting plugin uses this constant for their own purpose later? In that case showing deprecation notice might be confusing.
peterwilsoncc
left a comment
There was a problem hiding this comment.
LGTM, Thank you!
Review process: Search code for TAXONOMY_NAME and make sure it it prefixed throughout. It is.

Description of the Change
Constant TAXONOMY_NAME renamed to PODCASTING_TAXONOMY_NAME to avoid potential conflict. Deprecated notice added for older one.
Closes #228
Verification Process
Checklist:
Changelog Entry
Props @jayedul