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.

Sign up for GitHub

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

Jump to bottom

Enable an optional namespace parameter for hasAction & hasFilter #15362

Merged
merged 18 commits into from
Aug 26, 2019
Merged

Enable an optional namespace parameter for hasAction & hasFilter #15362

merged 18 commits into from
Aug 26, 2019

Conversation

adamsilverstein
Copy link
Member

@adamsilverstein adamsilverstein commented May 1, 2019 •
edited
Loading

Description

Core originally in from WordPress/packages#106
Fixes #7819

How has this been tested?

  • Add a hook with its required namespace.
  • Call matching hasfilter/hasAction and verify results correctly.
  • testing calling without namespace continues to work as expected.
  • See test files for various tests.

Types of changes

  • Add an optional namespace parameter for hasfilter & hasAction
  • Bdetter match core php functionshas_action has_filter which include this second parameter

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

aduth
aduth approved these changes May 22, 2019
View reviewed changes
packages/hooks/src/createHasHook.js Show resolved Hide resolved
@aduth
Copy link
Member

aduth commented May 22, 2019

There's a merge conflict to be resolved as well.

@aduth
Copy link
Member

aduth commented May 22, 2019 •
edited
Loading

Can we include a CHANGELOG.md note? I assume it should be publicly documented as well in README.md.

Separately, it would be good to see what it would take to automate the documentation using docgen like in other packages, so we don't need to maintain this documentation manually (tracked at #14227).

@swissspidy swissspidy added the [Package] Hooks /packages/hooks label May 28, 2019
@adamsilverstein adamsilverstein changed the title Enable optional namespace parameter for hasAction & hasFilter Enable an optional namespace parameter for hasAction & hasFilter Aug 12, 2019
@adamsilverstein
Copy link
Member Author

@gziolo This could use a review if you have time. I'm not sure why there are failing checks.

gziolo
gziolo approved these changes Aug 13, 2019
View reviewed changes
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

I left only nitpicks :)

It looks good and I think we should also update API Usage section in README file:
https://github.com/WordPress/gutenberg/tree/master/packages/hooks#api-usage

packages/hooks/CHANGELOG.md Outdated Show resolved Hide resolved
packages/hooks/src/test/index.test.js Outdated Show resolved Hide resolved
packages/hooks/src/test/index.test.js Outdated Show resolved Hide resolved
packages/hooks/src/test/index.test.js Show resolved Hide resolved
@gziolo
Update packages/hooks/CHANGELOG.md
bde71da 
Co-Authored-By: Grzegorz (Greg) Ziółkowski <grzegorz@gziolo.pl>
@adamsilverstein
Copy link
Member Author

@gziolo Thanks for the review. I will work on your feedback and update API Usage section in README.

@adamsilverstein
Copy link
Member Author

@gziolo I addressed your feedback and this should be ready for a merge once all tests pass.

talldan
talldan reviewed Aug 24, 2019
View reviewed changes
packages/hooks/README.md Outdated Show resolved Hide resolved
packages/hooks/README.md Outdated Show resolved Hide resolved
@gziolo @talldan
Apply suggestions from code review
51321b4 
Co-Authored-By: Daniel Richards <daniel.richards@automattic.com>
@gziolo gziolo added the [Type] Enhancement A suggestion for improvement. label Aug 26, 2019
@gziolo
Copy link
Member

gziolo commented Aug 26, 2019

Let's get this PR in, @adamsilverstein thanks for all iterations applied.

@gziolo gziolo merged commit 8eb5007 into master Aug 26, 2019
@gziolo gziolo deleted the fix-7601 branch August 26, 2019 11:00
@gziolo gziolo added this to the Gutenberg 6.4 milestone Aug 26, 2019
donmhico pushed a commit to donmhico/gutenberg that referenced this pull request Aug 27, 2019
@donmhico
Enable an optional namespace parameter for hasAction & hasFilter (Wor…
c2188c0 
…dPress#15362)

* from https://github.com/WordPress/packages/pull/106/files

* Update packages/hooks/src/createHasHook.js

Co-Authored-By: Pascal Birchler <pascal.birchler@gmail.com>

* clean up docblock spacing

* add changelog entry for new namespace parameter for hasAction & hasFilter

* Update packages/hooks/CHANGELOG.md

Co-Authored-By: Grzegorz (Greg) Ziółkowski <grzegorz@gziolo.pl>

* break out tests

* update readme

* Apply suggestions from code review

Co-Authored-By: Daniel Richards <daniel.richards@automattic.com>

* Update CHANGELOG.md
gziolo pushed a commit that referenced this pull request Aug 29, 2019
@gziolo
Enable an optional namespace parameter for hasAction & hasFilter (#15362
da806fd 
)

* from https://github.com/WordPress/packages/pull/106/files

* Update packages/hooks/src/createHasHook.js

Co-Authored-By: Pascal Birchler <pascal.birchler@gmail.com>

* clean up docblock spacing

* add changelog entry for new namespace parameter for hasAction & hasFilter

* Update packages/hooks/CHANGELOG.md

Co-Authored-By: Grzegorz (Greg) Ziółkowski <grzegorz@gziolo.pl>

* break out tests

* update readme

* Apply suggestions from code review

Co-Authored-By: Daniel Richards <daniel.richards@automattic.com>

* Update CHANGELOG.md
gziolo pushed a commit that referenced this pull request Aug 29, 2019
@gziolo
Enable an optional namespace parameter for hasAction & hasFilter (#15362
5e41987 
)

* from https://github.com/WordPress/packages/pull/106/files

* Update packages/hooks/src/createHasHook.js

Co-Authored-By: Pascal Birchler <pascal.birchler@gmail.com>

* clean up docblock spacing

* add changelog entry for new namespace parameter for hasAction & hasFilter

* Update packages/hooks/CHANGELOG.md

Co-Authored-By: Grzegorz (Greg) Ziółkowski <grzegorz@gziolo.pl>

* break out tests

* update readme

* Apply suggestions from code review

Co-Authored-By: Daniel Richards <daniel.richards@automattic.com>

* Update CHANGELOG.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@swissspidy swissspidy swissspidy left review comments

@talldan talldan talldan left review comments

@aduth aduth aduth approved these changes

@gziolo gziolo gziolo approved these changes

@youknowriad youknowriad Awaiting requested review from youknowriad

Assignees
No one assigned
Labels
[Package] Hooks /packages/hooks [Type] Enhancement A suggestion for improvement.
Projects
None yet
Milestone
Gutenberg 6.4
Development

Successfully merging this pull request may close these issues.

TRY: Support an optional namespace parameter for hasAction/hasFilter
5 participants
@adamsilverstein @aduth @gziolo @talldan @swissspidy