WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#26177 closed defect (bug) (duplicate)

Hooks Docs: wp-includes/class-wp-image-editor-imagick.php and wp-includes/class-wp-image-editor-gd.php

Reported by: theorboman Owned by: DrewAPicture
Milestone: Priority: normal
Severity: normal Version:
Component: Inline Docs Keywords: has-patch
Focuses: Cc:
PR Number:

Description

Documenting the 3 filter hooks in wp-includes/class-wp-image-editor-imagick.php and wp-includes/class-wp-image-editor-gd.php.

Because both classes use the same hooks I've included them both in this ticket. I'll add the documentation to the imagick class and will note in the gd class that the filters are documented there as per:

http://make.wordpress.org/core/handbook/inline-documentation-standards/php-documentation-standards/#4-1-duplicate-hooks.

Hope that's an ok way to do it :)

Attachments (1)

26177.patch (3.1 KB) - added by theorboman 6 years ago.
Patch for inline docs

Download all attachments as: .zip

Change History (10)

#1 @theorboman
6 years ago

  • Keywords has-patch added

@theorboman
6 years ago

Patch for inline docs

#2 @kpdesign
6 years ago

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

#3 @markoheijnen
6 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from reviewing to closed

Things will get changed a bit in this code by #25721 what has also some documentation. You are more then happy to change the inline docs of that patch.

#4 @kpdesign
6 years ago

  • Milestone set to Awaiting Review
  • Resolution duplicate deleted
  • Status changed from closed to reopened

This is not a duplicate of #25721, since only the hooks are being documented here.

I appreciate that your patch has recommended changes for the code and the docs in both those files, but until that patch is committed, the hooks need to be documented as the code is now.

Changes can be made to the patch here or the patch on #25721, depending on which patch is committed first.

@DrewAPicture will be reviewing this ticket, and I'm sure he'll take your patch into consideration.

#5 @kpdesign
6 years ago

  • Status changed from reopened to reviewing

#6 @markoheijnen
6 years ago

I disagree on that and I also don't understand why this wasn't communicated. That ticket should get committed for 3.8 so I don't see why it's needed.

#7 @markoheijnen
6 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from reviewing to closed

Sorry to close it again. I talked with nacin and the patch for #25721 is good to go in. I also updated that patch with the changes made here.

That said this ticket is a duplicate because it does addresses the same issue except #25721 does a bit more. In case the patch here would have committed before the other ticket then that patch need to be redone because of merge conflicts. Sometimes that just happens but in this case that would be weird.

#8 @DrewAPicture
6 years ago

Duplicate of #25721.

See comment:7

#9 @theorboman
6 years ago

Makes sense to me!

Note: See TracTickets for help on using tickets.