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

Docgen: Add @typedef and @property tags #18059

Merged
merged 2 commits into from Nov 14, 2019

Conversation

@Copons
Copy link
Contributor

Copons commented Oct 22, 2019

Description

  • Add support for @typedef and @property tags.
  • Fix the markdown of the SETTINGS_DEFAULT section in the Block Editor package readme.

Screenshots

Before After
Screenshot 2019-10-22 at 13 51 03 Screenshot 2019-10-24 at 16 44 41

Types of changes

Bug fix

@Copons Copons requested review from ellatrix, talldan and youknowriad as code owners Oct 22, 2019
@Copons Copons self-assigned this Oct 22, 2019
@ellatrix

This comment has been minimized.

Copy link
Member

ellatrix commented Oct 24, 2019

@Copons I think the readme file is automatically created. Perhaps the source should be updated?

@Copons

This comment has been minimized.

Copy link
Contributor Author

Copons commented Oct 24, 2019

@Copons I think the readme file is automatically created. Perhaps the source should be updated?

Oh damn! I had no idea! 🤦‍♂

Coming up with a new approach ASAP then!

@Copons Copons force-pushed the fix/block-editor-readme-settings-default branch from 24d78ac to 66b3f94 Oct 24, 2019
@Copons Copons changed the title Block Editor: Fix settings default section in package readme Docgen: Add @typedef and @property tags Oct 24, 2019
Copy link
Member

ellatrix left a comment

Looks good, though would appreciate an extra check from @nosolosw for the doc generation changes.

@nosolosw

This comment has been minimized.

Copy link
Member

nosolosw commented Oct 28, 2019

I think we can go ahead with this.

Some thoughts:

  • I've never seen @typedef being used within a declaration, but always as a standalone statement. Reading the JSDoc manual, I'm actually unsure how we're suppose to document objects. Using the property tag alone should be fine and typedef not required, but it's also natural to have the type definition within the object that first declares it.

  • Note that a task to add support for typedef already existed at #15186 It was more concerned with pulling the existing type definitions than this change, so I've updated the title to reflect better what it tracks.

@Copons

This comment has been minimized.

Copy link
Contributor Author

Copons commented Oct 28, 2019

@nosolosw To be honest, adding support for @typedef wasn't in my initial intentions.
But when I've built the docs with @property, I've noticed the WPEditorInserterItem changes, and they didn't really make much sense in the output readme without the @typedef giving a context to the @property list.

Now, this PR only takes into account the @typedef inside a function docs (I think?), but doesn't work for all those outside (e.g. this in the same file as WPEditorInserterItem).

I think that since this change doesn't really affect them, we could tackle those in a follow up though.

@vindl
vindl approved these changes Nov 14, 2019
@Copons Copons merged commit e3fca64 into master Nov 14, 2019
7 checks passed
7 checks passed
pull-request-automation
Details
Header rules - gutenberg-playground No header rules processed
Details
Pages changed - gutenberg-playground 3 new files uploaded
Details
Redirect rules - gutenberg-playground No redirect rules processed
Details
Mixed content - gutenberg-playground No mixed content detected
Details
Travis CI - Pull Request Build Passed
Details
netlify/gutenberg-playground/deploy-preview Deploy preview ready!
Details
@Copons Copons deleted the fix/block-editor-readme-settings-default branch Nov 14, 2019
@youknowriad youknowriad added this to the Gutenberg 7.0 milestone Nov 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.