Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds extra parameters to dt_create_missing_terms. #378

Merged
merged 2 commits into from
Jul 17, 2019

Conversation

mmcachran
Copy link
Contributor

Description of the Change

Add taxonomy and term data to dt_create_missing_terms filter to allow whitelisting/blacklisting taxonomies/terms.

@mmcachran mmcachran added the type:enhancement New feature or request. label May 10, 2019
@jeffpaul jeffpaul added this to the 1.5.0 milestone May 13, 2019
@jeffpaul jeffpaul requested a review from helen May 13, 2019 16:26
@jeffpaul
Copy link
Member

Resolves #379

@jeffpaul jeffpaul requested review from adamsilverstein and dkotter and removed request for helen June 6, 2019 18:26
@jeffpaul
Copy link
Member

jeffpaul commented Jun 6, 2019

@adamsilverstein @dkotter if either of you have time to review this that would be great, thanks!

@helen
Copy link
Contributor

helen commented Jun 13, 2019

Leaving a review note: I just want to make sure that this doesn't impact existing hooked functions - there's a change in PHP 7 that can throw notices/errors if the number of args doesn't match but I don't remember off the top of my head what exactly causes the issue. If you do know, that'd be great, but otherwise I will look into this in the next week or two.

includes/utils.php Outdated Show resolved Hide resolved
@adamsilverstein
Copy link

there's a change in PHP 7 that can throw notices/errors if the number of args doesn't match

@helen i couldn't find that change although i vaguely recall it. In any case, I think its only an issue if your function expects more parameters than are passed (and provides no defaults)? In that case, this change would be safe as its adding arguments so existing handlers would work as expected.

Copy link
Contributor

@helen helen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making suggestions so I can commit them :) Turns out the commit suggestions feature doesn't work for forks? Anyway, I committed them directly in Git.

includes/utils.php Outdated Show resolved Hide resolved
includes/utils.php Outdated Show resolved Hide resolved
includes/utils.php Outdated Show resolved Hide resolved
@helen helen merged commit f7821ee into 10up:develop Jul 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:enhancement New feature or request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants