WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#26870 closed task (blessed) (fixed)

Add (JSDoc)umentation to the Backbone-centric Media files

Reported by: wonderboymusic Owned by:
Milestone: 3.9 Priority: normal
Severity: normal Version: 3.5
Component: Media Keywords: needs-patch needs-docs
Focuses: docs Cc:

Description

See: http://make.wordpress.org/core/2014/01/13/audio-video-2-0-codename-disco-fries/

This is a spike ticket for feedback and commits about JSDoc implementation inside the @koop Media code from 3.5

Attachments (5)

26870-media-views.diff (91.7 KB) - added by wonderboymusic 3 years ago.
26870-media-models.diff (24.0 KB) - added by wonderboymusic 3 years ago.
26870-media-editor.diff (16.7 KB) - added by wonderboymusic 3 years ago.
26870-media-views-02.patch (5.2 KB) - added by gcorne 3 years ago.
26870.diff (509 bytes) - added by kovshenin 3 years ago.

Download all attachments as: .zip

Change History (33)

#1 @wonderboymusic
3 years ago

26870-media-views.diff is a first pass at documenting src/wp-includes/js/media-views.js. It would be an understatement to say I almost blacked out several times chasing down all of the chains of inheritance. There is still more to document, but I think this could go in for a base to work from.

Notes:

  • THE most important thing is the inheritance chain of each class with a constructor. Many of them inherit from classes 6 levels deep and mix-in wp.media.controller.StateMachine just for laughs. These classes are marked with @constructor, and the classes in the inheritance chain are annotated with @augments
  • The next most important step for a lot of this code is documenting this inside of instance methods with the @this annotation. This is by far the most confusing aspect of a lot of the code.
  • wp.media.controller.State is basically an abstract class that many of these classes are intertwined with.
  • wp.media.controller.Region provides a lot of the black voodoo that triggers custom events

Anyways - more to be done.

Last edited 3 years ago by wonderboymusic (previous) (diff)

#2 @wonderboymusic
3 years ago

26870-media-models.diff adds initial JSDoc blocks for src/wp-includes/js/media-models.js - this is a base to work from and iterate on

#3 @wonderboymusic
3 years ago

26870-media-editor.diff​ add baseline docs to src/wp-includes/js/media-editor.js - will need more iterations, but this gets us started.

#4 @wonderboymusic
3 years ago

In 26984:

Add initial JSDoc blocks to media-editor.js. The initial blocks are a baseline to work from and invite future iterations. Initial commit is to avoid commits this large in the future.

See #26870.

#5 @wonderboymusic
3 years ago

In 26985:

Add initial JSDoc blocks to media-models.js. The initial blocks are a baseline to work from and invite future iterations. Initial commit is to avoid commits this large in the future.

See #26870.

#6 @markoheijnen
3 years ago

Was the first part of [26985] intended? Little bit clueless here. To me this is a code change and not documentation so I rather would have seen two commits to make it clear why it was done.

#7 @wonderboymusic
3 years ago

There are several places where it is hard to disambiguate / add inline docs, due to missing {}s around if/elses - no longer

#8 @wonderboymusic
3 years ago

In 26987:

Elaborate on JSDoc blocks for media-editor.js.

See #26870.

#9 @wonderboymusic
3 years ago

In 26988:

Correct a type.

See #26870.

#10 @wonderboymusic
3 years ago

In 26990:

Remove debug cruft.

Props nacin.
See #26870.

#11 @wonderboymusic
3 years ago

In 26991:

Make some @param types more specific in media-models.js.

See #26870.

#12 @wonderboymusic
3 years ago

In 27000:

Add the @namespace annotation where appropriate in media-editor.js. Also indicate the default value of a few params via the [options={}] syntax.

See #26870.

#13 @wonderboymusic
3 years ago

In 27005:

Add inline docs to each method of the namespaced objects in media-editor.js, explaining what each does. Add inline docs to params as well.

See #26870.

#14 @wonderboymusic
3 years ago

In 27031:

Add some JSDoc annotations to media-models.js to disambiguate instance properties and static class properties/methods in the base media Models.

See #26870.

#15 @nacin
3 years ago

  • Focuses docs added

Rock.

#16 @wonderboymusic
3 years ago

In 27032:

Add some inline docs to methods in media-models.js - particularly around @param and @returns.

See #26870.

This ticket was mentioned in IRC in #wordpress-ui by joedolson. View the logs.


3 years ago

This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.


3 years ago

#19 @gcorne
3 years ago

While doing a bit of jshint work on media-views.js, I noticed:

/**
  * call 'validator' directly on wp.media.model.Selection
  */
&& media.model.Selection.prototype.validator.apply( this, arguments );

@wonderboymusic, I am not sure it makes sense to document what is standard, albeit ugly, javascript. This particular case also breaks the jshint rules and makes the logical expression a bit harder to read, which is why it jumped out to me. If we feel like these statements are not clear, maybe we want to introduce a _super() method. I have seen but not used the following approach for doing this, which we probably would want to add only at the wp.Backbone or wp.media level.

Backbone.Model.prototype._super = function(funcName){
    return this.constructor.__super__[funcName].apply(this, _.rest(arguments));
};

#20 @wonderboymusic
3 years ago

yeahhh... some of that is overzealous - so much of that code was a black hole when I started, the extra comments were more for my own comfort than anything else. Feel free to make whatever edits you feel are necessary - I just wanted to get the ball rolling so I understood how all of it worked.

#21 @gcorne
3 years ago

26870-media-views-02.patch removes the comments when calling super via Function.prototype.apply and adds a bit of context to the documentation for some of the base constructors.

#22 @wonderboymusic
3 years ago

In 27126:

Add some inline docs to media-views.js and remove some unnecessary comments from a few super calls.

Props gcorne.
See #26870.

This ticket was mentioned in IRC in #wordpress-dev by TomAuger. View the logs.


3 years ago

@kovshenin
3 years ago

#25 @wonderboymusic
3 years ago

In 27309:

Correct a misspelled property name in a media-views.js annotation.

Props kovshenin.
See #26870.

#26 @nacin
3 years ago

  • Type changed from enhancement to task (blessed)

Anything big left here?

This ticket was mentioned in IRC in #wordpress-dev by wonderboymusic. View the logs.


3 years ago

#28 @nacin
3 years ago

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.