Skip to content

/gutenberg

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

Docs: Clarify "Experimental and Unstable APIs" guidelines #14557

Merged
merged 1 commit into from Mar 26, 2019
+18 −8

Conversation

Reviewers

@gziolo gziolo

@nerrad nerrad

@mkaz mkaz

@youknowriad youknowriad

@nosolosw nosolosw

@ajitbohra ajitbohra

@chrisvanpatten chrisvanpatten

@notnownikki notnownikki

Assignees
No one assigned
Labels
[Type] Documentation
Projects
None yet
Milestone
  5.4 (Gutenberg)
5 participants
@aduth @nosolosw @gziolo @nerrad @youknowriad
@aduth
Copy link
Member

commented Mar 21, 2019

This pull request seeks to clarify the wording of the Coding Guidelines "Experimental and Unstable APIs" section, toward the goals of:

  • Providing clearer messaging to potential external consumers that there is no support commitment
  • Better contrasting the specific differences between what constitutes an API as being "experimental" vs. "unstable"
  • Noting that an experimental API must inherently be part of a breaking change as part of its stabilization (by the fact that __experimental is to be removed from its exposed name)
  • Emphasizing that these APIs should be short-lived

@aduth aduth added the [Type] Documentation label Mar 21, 2019

@aduth aduth requested review from ajitbohra, chrisvanpatten, gziolo, mkaz, nosolosw, notnownikki and youknowriad as code owners Mar 21, 2019

@nerrad

nerrad approved these changes Mar 21, 2019
View changes

Copy link
Contributor

left a comment

In general I like these proposed changes (hence the approval). How does this impact the automatic docs generation? I think I've noticed a few cases where auto-docs generation has picked up prefixed functions and thus would violate these guidelines. It's possible that's already been rectified somewhere but just thought I'd bring it up.
@aduth

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented Mar 21, 2019

How does this impact the automatic docs generation?

By default, it's meant to ignore anything containing these keywords:

gutenberg/bin/update-readmes.js

Line 52 in 98a19b6

'--ignore "unstable|experimental"',

I think I've noticed a few cases where auto-docs generation has picked up prefixed functions and thus would violate these guidelines. It's possible that's already been rectified somewhere but just thought I'd bring it up.

I'll defer to @nosolosw on this.
@gziolo

gziolo reviewed Mar 22, 2019
View changes

docs/contributors/coding-guidelines.md

```js
export { __unstableDoAction } from './api';
export { __experimentalDoExcitingExperimentalAction } from './api';
export { __unstableDoTerribleAwfulAction } from './api';

This comment has been minimized.

Sign in to view
Copy link
@gziolo

gziolo Mar 22, 2019

Member

Love this example ❤️

Related, is there any way of telling ESLint that __exeperminental* and __unstable* prefixes are fine? It yells when you try to use. It is fine but a little bit annoying to use such method. At least for experimental, it could be less strict.

This comment has been minimized.

Sign in to view
Copy link
@aduth

aduth Mar 25, 2019

Author Member

Related, is there any way of telling ESLint that __exeperminental* and __unstable* prefixes are fine? It yells when you try to use. It is fine but a little bit annoying to use such method. At least for experimental, it could be less strict.

Could you share an example? I think I've seen it before where we use the non-conventional unstable__Whatever, but with the correct, leading underscores, it doesn't trip ESLint's camelcase rule:

https://eslint.org/demo/#eyJ0ZXh0IjoiY29uc3QgeyBfX2V4cGVyaW1lbnRhbEZvbyB9ID0gd2luZG93O1xuY29uc3QgeyBleHBlcmltZW50YWxfX0ZvbyB9ID0gd2luZG93OyIsIm9wdGlvbnMiOnsicGFyc2VyT3B0aW9ucyI6eyJlY21hVmVyc2lvbiI6OSwic291cmNlVHlwZSI6InNjcmlwdCIsImVjbWFGZWF0dXJlcyI6e319LCJydWxlcyI6eyJjYW1lbGNhc2UiOjJ9LCJlbnYiOnt9fX0=

This comment has been minimized.

Sign in to view
Copy link
@gziolo

gziolo Mar 26, 2019

Member

https://github.com/WordPress/gutenberg/pull/14551/files#diff-33e0ac9c44b16a617039a563d4c065f3R60

It looks like I followed the wrong usage (unstable__bootstrapServerSideBlockDefinitions) in the first place 🤣
@gziolo

gziolo approved these changes Mar 22, 2019
View changes

Copy link
Member

left a comment •
edited

I like all clarifications included in this PR.

I was wondering whether we should replace unstable keyword with something scarier:

export { __WILL_BE_REMOVED_doTerribleAwfulAction } from './api';

I'm not sure about the correct phrasing but sharing how it could be visually explained to the observers who won't read docs but will feel a strong desire to use such API method :)
@nosolosw

This comment has been minimized.

Sign in to view
Copy link
Member

commented Mar 22, 2019 •
edited

I think I've noticed a few cases where auto-docs generation has picked up prefixed functions and thus would violate these guidelines. It's possible that's already been rectified somewhere but just thought I'd bring it up.

Just checked for unstable/experimental in Markdown files, and this is what I've found.

experimental:

  • docs/designers-developers/developers/data/data-core-annotations.md
  • docs/designers-developers/developers/data/data-core-editor.md

unstable:

  • docs/designers-developers/developers/data/data-core-block-editor.md
  • docs/designers-developers/developers/data/data-core-editor.md
  • packages/block-editor/README.md (UnstableRichTextInputEvent)

Of those, only block-editor has set up auto-generated docs at the moment. I'll get to fix that export (edit: see #14565).

Please, if you see any export that shouldn't be auto-documented, ping me and I'll get to fix it! :)

@nosolosw nosolosw referenced this pull request Mar 22, 2019

Merged

Make prefix match case insensitive #14565

@aduth

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented Mar 25, 2019

I was wondering whether we should replace unstable keyword with something scarier:

There was a previous Core JS agenda item on this exact point:

There was a conversation recently on Github where it became evident that marking an API as unstable is not sufficient enough in its own right to scare people away from using them (and subsequently being frustrated that they’re not documented).

https://make.wordpress.org/core/2019/01/29/javascript-chat-summary-january-29-2019/

In the meeting, it seemed generally agreed upon that there was not a need for changing the name.

I'm happy to revisit it, but also think that (a) changing conventions is painful, (b) there's no perfect solution and the current convention is a reasonable middle-ground and (c) with new encouragement as of this pull request, unstable APIs should be very short-lived and not commonly encountered.
@gziolo

This comment has been minimized.

Sign in to view
Copy link
Member

commented Mar 26, 2019

I'm happy to revisit it, but also think that (a) changing conventions is painful, (b) there's no perfect solution and the current convention is a reasonable middle-ground and (c) with new encouragement as of this pull request, unstable APIs should be very short-lived and not commonly encountered.

I totally get your point and respect the previous decision made. It's fine to proceed as is and that's why I added my ✅ Ideally we don't have to use these prefixes at all rather than spend too much time thinking how to name them 😅

@gziolo gziolo merged commit 7ec933b into master Mar 26, 2019
1 check passed

1 check passed

Travis CI - Pull Request Build Passed
Details

@gziolo gziolo deleted the update/coding-guidelines-unstable-apis branch Mar 26, 2019

@youknowriad youknowriad added this to the 5.4 (Gutenberg) milestone Mar 29, 2019

This was referenced Apr 1, 2019

Merged
Merged

@aduth aduth referenced this pull request Apr 25, 2019

Open

Build Tooling: Update use of docgen to target only unstable APIs named by convention #15197

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.