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

tweak: add new action hooks and deprecated existing #1024

Merged
merged 15 commits into from
Apr 18, 2023

Conversation

ravinderk
Copy link
Contributor

@ravinderk ravinderk commented Mar 7, 2023

Description of the Change

As discussed, I deprecated dt_push_post and added two action hooks, dt_push_external_post and dt_push_network_post.

Closes #123

How to test the Change

  • A notice should log in debug.log when use dt_push_post action hook
  • dt_push_external_post and dt_push_network_post action hooks should not cause any issue when use

Changelog Entry

Added - dt_push_external_post and dt_push_network_post action hooks.
Deprecated - dt_push_post action hook

Credits

Props @dkotter @peterwilsoncc @dhanendran

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

@ravinderk
Copy link
Contributor Author

ravinderk commented Mar 24, 2023

While checking existing action/filters hooks, I notice the following things:

  • We only document one filter/action hook. As a developer, I notice the following issues:
    • I saw that in a few hooks, we pass class objects, meaning object types often differ, which can cause a fatal error. We should mention it in the hooks doc.

    • We show only one location of the filter doc. I think if we reuse the same filter at multiple locations and then mention this, it will be helpful for the developer

      List of a few hooks:

      • dt_push_post_args
      • dt_pull_post_terms
  • We should mention multiple value types in the filter doc's return type. I noticed that dt_remote_get filter hook returns WP_Post , array, and boolean

Let me know if you want me to take action on the same pull request.

cc: @peterwilsoncc @dkotter

@ravinderk ravinderk marked this pull request as ready for review March 24, 2023 11:18
@ravinderk ravinderk requested a review from a team as a code owner March 24, 2023 11:18
@ravinderk ravinderk requested review from peterwilsoncc and removed request for a team March 24, 2023 11:18
@ravinderk ravinderk changed the title tweak: add new filters and deprecated existing tweak: add new action hooks and deprecated existing Mar 24, 2023
Copy link
Collaborator

@peterwilsoncc peterwilsoncc left a comment

Choose a reason for hiding this comment

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

A few minor notes inline.

ravinderk and others added 2 commits April 18, 2023 10:27
…on.php

Co-authored-by: Peter Wilson <519727+peterwilsoncc@users.noreply.github.com>
Co-authored-by: Peter Wilson <519727+peterwilsoncc@users.noreply.github.com>
@ravinderk
Copy link
Contributor Author

@peterwilsoncc I merged your suggestion and replied to your question. This pull request is ready to review.

Copy link
Collaborator

@peterwilsoncc peterwilsoncc left a comment

Choose a reason for hiding this comment

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

Thanks, let's figure out the other hooks you mentioned on a follow up PR.

@peterwilsoncc peterwilsoncc merged commit 7f3a3f9 into develop Apr 18, 2023
@peterwilsoncc peterwilsoncc deleted the fix/add-hooks-123 branch April 18, 2023 05:25
@peterwilsoncc peterwilsoncc restored the fix/add-hooks-123 branch April 18, 2023 05:25
@peterwilsoncc peterwilsoncc deleted the fix/add-hooks-123 branch April 18, 2023 05:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pass along identical arguments in dt_push_post actions
3 participants