Perfect your code
With built-in code review tools, GitHub makes it easy to raise the quality bar before you ship. Join the 36 million developers who've merged over 200 million pull requests.
Sign up for free See pricing for teams and enterprisesDocs: Clarify "Experimental and Unstable APIs" guidelines #14557
Conversation
aduth
added
the
[Type] Documentation
label
Mar 21, 2019
aduth
requested review from
ajitbohra,
chrisvanpatten,
gziolo,
mkaz,
nosolosw,
notnownikki and
youknowriad
as
code owners
Mar 21, 2019
nerrad
approved these changes
Mar 21, 2019
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. |
This comment has been minimized.
This comment has been minimized.
By default, it's meant to ignore anything containing these keywords: gutenberg/bin/update-readmes.js Line 52 in 98a19b6
I'll defer to @nosolosw on this. |
gziolo
reviewed
Mar 22, 2019
|
||
```js | ||
export { __unstableDoAction } from './api'; | ||
export { __experimentalDoExcitingExperimentalAction } from './api'; | ||
export { __unstableDoTerribleAwfulAction } from './api'; |
This comment has been minimized.
This comment has been minimized.
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.
This comment has been minimized.
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:
This comment has been minimized.
This comment has been minimized.
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
approved these changes
Mar 22, 2019
I like all clarifications included in this PR. I was wondering whether we should replace 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 :) |
This comment has been minimized.
This comment has been minimized.
Just checked for unstable/experimental in Markdown files, and this is what I've found. experimental:
unstable:
Of those, only Please, if you see any export that shouldn't be auto-documented, ping me and I'll get to fix it! :) |
This comment has been minimized.
This comment has been minimized.
There was a previous Core JS agenda item on this exact point:
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. |
This comment has been minimized.
This comment has been minimized.
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 |
aduth commentedMar 21, 2019
This pull request seeks to clarify the wording of the Coding Guidelines "Experimental and Unstable APIs" section, toward the goals of:
__experimental
is to be removed from its exposed name)