WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#26894 closed defect (bug) (fixed)

Hook Docs (6): wp-admin/includes/export.php

Reported by: cmmarslender Owned by: kpdesign
Milestone: 3.9 Priority: normal
Severity: normal Version:
Component: Export Keywords: has-patch commit
Focuses: docs Cc:

Description


Attachments (4)

export.diff (2.6 KB) - added by cmmarslender 7 years ago.
Inline Docs
export.2.diff (2.6 KB) - added by cmmarslender 7 years ago.
26894.diff (2.7 KB) - added by DrewAPicture 7 years ago.
3rd pass
26894.1.diff (2.7 KB) - added by kpdesign 7 years ago.
Fourth pass

Download all attachments as: .zip

Change History (15)

#1 @DrewAPicture
7 years ago

  • Keywords needs-patch added
  • Summary changed from Hooks Docs: wp-admin/includes/export.php to Hook Docs: wp-admin/includes/export.php

#2 @DrewAPicture
7 years ago

  • Summary changed from Hook Docs: wp-admin/includes/export.php to Hook Docs (6): wp-admin/includes/export.php

@cmmarslender
7 years ago

Inline Docs

#3 @cmmarslender
7 years ago

  • Keywords needs-patch removed

#4 @cmmarslender
7 years ago

  • Keywords has-patch added

#5 @DrewAPicture
7 years ago

  • Owner set to DrewAPicture
  • Status changed from new to reviewing

Chris: Welcome to Trac, and thanks for the patch. I'll review it and get back to you with any notes.

#6 @DrewAPicture
7 years ago

  • Keywords needs-patch added; has-patch removed

Thanks for the patch. Here are some notes on export.diff:

Overall:

  • All short descriptions and parameter descriptions need periods at the end.

export_wp hook:

  • I'd probably explicitly mention 'WXR export' in the description instead of just 'export'
  • The $args parameter description isn't really accurate since you don't "filter" anything -- and it doesn't help that the parameter description for the function is wrong. Just be more direct with the description, as it's passed to the action. Maybe something like: "An array of export arguments."

the_content_export,
the_excerpt_export filters:

  • Rather than denote filtering the value "during" an export, perhaps we should go with "used for exports" or something

wxr_export_skip_postmeta filter:

  • Again "used for exports" instead of "during"
  • The $skip parameter should specify the default.
  • All parameter descriptions should end with periods.

#7 @cmmarslender
7 years ago

  • Keywords has-patch added; needs-patch removed

Thanks for the feedback. I've uploaded a new version of the patch!

@DrewAPicture
7 years ago

3rd pass

#8 @DrewAPicture
7 years ago

  • Milestone changed from Awaiting Review to 3.9
  • Owner changed from DrewAPicture to kpdesign

Looks pretty good. I made a few adjustments to export.2.diff in 26894.diff -- including that the $meta parameter in the wxr_export_skip_postmeta filter is an object not a string.

Thanks for the effort. This needs a secondary review and recommendation.

#9 @jeremyfelt
7 years ago

  • Component changed from Inline Docs to Export
  • Focuses docs added

@kpdesign
7 years ago

Fourth pass

#10 @kpdesign
7 years ago

  • Keywords commit added

26894.1.diff contains a few minor changes for consistency.

This one's ready to go in. Recommend commit.

#11 @DrewAPicture
7 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 27048:

Inline documentation for hooks in wp-admin/includes/export.php.

Props cmmarslender, kpdesign.
Fixes #26894.

Note: See TracTickets for help on using tickets.