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

Data: Documentation: Document undocumented declarations #15176

Merged
merged 2 commits into from May 31, 2019

Conversation

aduth
Copy link
Member

@aduth aduth commented Apr 25, 2019

This pull request seeks to add missing documentation for data module exports:

  • plugins
  • use
  • RegistryConsumer
  • RegistryProvider

Implementation notes:

The plugins documentation here could be improved to better reference available options. However, it's not clear how to reference this without adding overhead to future updates to these plugins, which would likely be overlooked. Suggestions welcome.

Testing Instructions:

Verify by rendered output of "Files changed" tab that produced documentation is sensible.

There are only documentation changes. Thus, there is no expected impact on application runtime.

* A context consumer component which provides to the rendered child function a
* reference to the current registry context.
*
* @link https://reactjs.org/docs/context.html#contextconsumer
Copy link
Member

@oandregal oandregal Apr 25, 2019

Choose a reason for hiding this comment

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

Prefer the @see tag (see)

Copy link
Member Author

Choose a reason for hiding this comment

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

Prefer the @see tag (see)

👍 Updated in c10bcab

export { createRegistrySelector, createRegistryControl } from './factory';

/**
Copy link
Member

Choose a reason for hiding this comment

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

I wish we had a better way to auto-document this. Unfortunately, we don't yet.

@oandregal
Copy link
Member

It looks fine to me, but I defer to other people with more data-foo in case they have any input about the text itself.

@aduth aduth force-pushed the update/data-undocumented branch from c10bcab to 2e82ebf Compare May 28, 2019 19:20
@aduth
Copy link
Member Author

aduth commented May 28, 2019

Rebased to account for conflicts from #15737, dropping the now-redundant documentation for the registry consumer and provided (previously 1786274).

@nerrad Would you be able to give this a look?

Copy link
Contributor

@nerrad nerrad left a comment

Choose a reason for hiding this comment

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

This is better than what we have currently (certainly better than "undocumented") so I'm in favor of merging this for now and it can be iterated on if needed?

@aduth aduth merged commit 04ca6b4 into master May 31, 2019
@aduth aduth deleted the update/data-undocumented branch May 31, 2019 20:35
@youknowriad youknowriad added this to the Gutenberg 5.9 milestone Jun 7, 2019
sbardian pushed a commit to sbardian/gutenberg that referenced this pull request Jul 29, 2019
)

* Data: Documentation: Document plugins

* Data: Documentation: Document use
dd32 pushed a commit to dd32/gutenberg that referenced this pull request Sep 27, 2019
)

* Data: Documentation: Document plugins

* Data: Documentation: Document use
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Data /packages/data [Type] Developer Documentation Documentation for developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants