Opened 8 years ago
Last modified 5 years ago
#36946 new defect (bug)
Provide `id` properties on core objects
Reported by: | jeremyfelt | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | |
Component: | General | Keywords: | needs-patch |
Focuses: | Cc: |
Description
In #36717, WP_Site
and WP_Network
will get magic getters/setters so that id
and other more properly named properties can be used.
We should standardize on id
rather than ID
for core objects where it makes sense.
Current primary IDs for objects are:
WP_Comment
->comment_ID
WP_Post
->ID
WP_Term
->term_id
WP_User
->ID
WP_Widget
->id
WP_Screen
->id
WP_User
is somewhat unique because the property was id
and then deprecated in [18504] in favor of ID
. We should be able to un-deprecate this.
Change History (7)
#2
in reply to:
↑ 1
;
follow-ups:
↓ 3
↓ 7
@
8 years ago
Replying to DrewAPicture:
My vote would be on standardizing on ID as it's actually the abbreviation for identifier and is what we (correctly) use all over the place in inline docs.
'id' makes me think of Freud. :-)
Here's the related discussion from today's core meeting. Hopefully to sway you a bit. :)
Also worth noting that the WP REST API team is using id
in responses for core objects.
#3
in reply to:
↑ 2
;
follow-up:
↓ 4
@
8 years ago
Replying to jeremyfelt:
Replying to DrewAPicture:
My vote would be on standardizing on ID as it's actually the abbreviation for identifier and is what we (correctly) use all over the place in inline docs.
'id' makes me think of Freud. :-)
Here's the related discussion from today's core meeting. Hopefully to sway you a bit. :)
Also worth noting that the WP REST API team is using
id
in responses for core objects.
Interesting. Of course it also doesn't help that we're sorely inconsistent at the db level either:
- commentmeta:
meta_id
,comment_id
- comments:
comment_ID
,comment_post_ID
,user_id
- options:
option_id
- postmeta:
meta_id
,post_id
- posts:
ID
- termmeta:
meta_id
,term_id
- terms:
term_id
- term_relationships:
object_id
,term_taxonomy_id
- term_taxonomy:
term_taxonomy_id
,term_id
- usermeta:
umeta_id
,user_id
- users:
ID
Pretty much the only thing that's consistent there is that when it's 'ID' alone, it's 'ID'. Yuck.
All that aside, per the linked discussion, I really don't think a deprecated notice or a _doing_it_wrong()
is a good idea, considering our history and the fact that $user->ID
and $post->ID
are used all over the WordPress ecosystem.
#4
in reply to:
↑ 3
;
follow-up:
↓ 5
@
8 years ago
Replying to DrewAPicture:
All that aside, per the linked discussion, I really don't think a deprecated notice or a
_doing_it_wrong()
is a good idea, considering our history and the fact that$user->ID
and$post->ID
are used all over the WordPress ecosystem.
I really agree with this, I don't think we should be firing deprecated notices for everything but rather just have the different-case'd variants "just work". There's zero harm in using ->ID
over ->id
, it's not like we're ever going to remove the ID
versions.
#5
in reply to:
↑ 4
@
8 years ago
Replying to dd32:
Replying to DrewAPicture:
All that aside, per the linked discussion, I really don't think a deprecated notice or a
_doing_it_wrong()
is a good idea, considering our history and the fact that$user->ID
and$post->ID
are used all over the WordPress ecosystem.
I really agree with this, I don't think we should be firing deprecated notices for everything but rather just have the different-case'd variants "just work". There's zero harm in using
->ID
over->id
, it's not like we're ever going to remove theID
versions.
Agreed. I don't think we need to deprecate anything. Just un-deprecate WP_User::id
. :)
#6
@
8 years ago
I was going to +1 ID, but the slack log convinced me otherwise...
Is it worth discussing function and hook names at the same time? get_the_ID(), the_ID(), get_author_user_ids, get_current_blog_id().
Seems like it's only the oldest of functions using ID.
#7
in reply to:
↑ 2
@
8 years ago
Replying to jeremyfelt:
Also worth noting that the WP REST API team is using
id
in responses for core objects.
We switched away from uppercase ID
because it's inconsistent with every other field. We have properties like display_name
, post_title
, etc, but ID
doesn't match this. It looks especially weird when you have things like site_ID
. Plus: every other abbreviation is lowercase, except for ID
. Under the current system, you could end up with http_url_ID
, which makes 0 sense.
While @DrewAPicture is correct that "ID" is the correct casing for the English abbreviation, we're not using regular English sentences, we're using snake_case, which is all lowercase.
Replying to jb510:
Is it worth discussing function and hook names at the same time? get_the_ID(), the_ID(), get_author_user_ids, get_current_blog_id().
PHP function names are case-insensitive, so I think it makes sense to re-case all these at the same time, since it won't break anything. Docs are the main thing that will need updating.
My vote would be on standardizing on ID as it's actually the abbreviation for identifier and is what we (correctly) use all over the place in inline docs.
'id' makes me think of Freud. :-)