Make WordPress Core

Opened 18 months ago

Last modified 11 months ago

#57586 new enhancement

term_exists() return type not consistent regarding wp_insert_term()

Reported by: hugod's profile hugod Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.0
Component: Taxonomy Keywords: has-patch dev-feedback early needs-dev-note needs-testing
Focuses: Cc:

Description

term_exists() returns an array of strings containing term_id and term_taxonomy_id. Although wp_insert_term() as well as wp_update_term() return an array of integers.
For consistency, it'd be better to return alway the same type. Also, it'd be less error prone.

Change History (13)

This ticket was mentioned in PR #3943 on WordPress/wordpress-develop by @hugod.


18 months ago
#1

  • Keywords has-patch added

Trac ticket: #57586

#2 @spacedmonkey
18 months ago

Thanks for the ticket @hugod.

Is there a worry here that these might be a breaking change? Would changing this type have any side effects that were unexpected?

#3 @mukesh27
18 months ago

  • Keywords dev-feedback added
  • Version set to 3.0

Hi there, Thanks for ticket.

On Changeset the document updated.

#36949 related ticket in which string was added.

#4 @peterwilsoncc
18 months ago

I believe there have been similar type changes in the past.

From memory get_terms() used to return the ID as a string and it was changed to an integer some time around WordPress 4.4. The details may be a little incorrect, it was 8 years ago 🙂.

Further type changes were made for consistency in #53235. As PHP becomes stricter, it seems like a step in the right direction.

Provided there is a dev-note with sufficient warning, I think it's a change that can be made early in the release cycle. Probably for a future release rather than for WordPress 6.2

#5 @hugod
18 months ago

Hi there,
@spacedmonkey It changes return type of both category_exists and tag_exists. But I updated the doc only for category_exists.
The one in tag_exists has @return mixed. Should I add a more accurate doc for it?

#6 @hugod
14 months ago

Hi folks!
I just wanted to know if there any chance the ticket will land in 6.3 backlog.
Tell me if the PR need any change.

#7 @spacedmonkey
14 months ago

@hugod This change was not marked in the milestone of 6.3. So there is no current plan. I like the change personally. But it is a B/C break. I will defer to the wise @peterwilsoncc . If he like the change, that I will commit it.

#8 @peterwilsoncc
13 months ago

  • Keywords early needs-dev-note added
  • Milestone changed from Awaiting Review to 6.4

@spacedmonkey I think it's a good idea, let's do it early in the next cycle.

#9 @hellofromTonya
12 months ago

I'm doing an audit of all 6.4 early tickets to determine if each should or shouldn't be in the early queue.

The handbook explains the `early` keyword as:

This keyword should only be applied by committers. The keyword signals that the ticket is a priority, and should be handled early in the next release cycle.

early means the commits need to happen during the early part of the Alpha cycle; else, the ticket gets bumped out of the current milestone.

Is this ticket an early candidate?

Yes. Why? There are mentions of a potential BC (backward compatible) break. If yes, then careful consideration is needed and commit(s) should be done as early as possible to give a longer period of time for extender awareness, feedback, and testing.

#10 @oglekler
11 months ago

  • Keywords needs-testing added

This ticket was mentioned in Slack in #core by oglekler. View the logs.


11 months ago

#12 @oglekler
11 months ago

This is an early ticket, and it is too late for 6.4, so this ticket need to be rescheduled for 6.5.

#13 @peterwilsoncc
11 months ago

  • Milestone changed from 6.4 to Future Release
Note: See TracTickets for help on using tickets.