-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Add API docs to e2e-test-utils package #13856
Conversation
> node packages/docgen/src/cli.js packages/e2e-test-utils/src/index.js --append API --output packages/e2e-test-utils/README.md Note that you first need to copy manually the docgen package to this branch for it to be available.
This is amazing 🔮 I wil take a closer look tomorrow ☀️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left my thoughts. I don't see any blockers, mostly ideas on how to iterate further and make maintenance easier. Let me know what do you think and whether you plan to apply any changes to this PR.
packages/e2e-test-utils/README.md
Outdated
@@ -11,3 +11,528 @@ npm install @wordpress/e2e-test-utils --save-dev | |||
``` | |||
|
|||
<br/><br/><p align="center"><img src="https://s.w.org/style/images/codeispoetry.png?1" alt="Code is Poetry." /></p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noting that this logo is usual at the end of the file.
This made me think that maybe we simplify this process a bit and we only reference API file in the file and generate the document inside docs/api.md
file or something like that. The idea would be that we only have to update one file which can be completely erased every time it gets updated. Another way of handling it is to come up with some start and end token which you put inside readme file and they get overriden with the docs script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/WordPress/gutenberg/pull/13329/files#r256816506 is another case where using tokens instead of using section titles would be beneficial.
|
||
### activatePlugin | ||
|
||
[src/index.js#L1-L1](src/index.js#L1-L1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is cool, however, in the iteration 2 I would love to see it traversing to the actual source code ✨
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about that as well. We already look up the symbol declaration in many cases to retrieve the JSDoc; not in all cases, though. It'd be a nice addition, and I wonder if perhaps we should add both (export statement + symbol declaration). Undecided what would be best, but it's doable.
|
||
Activates an installed plugin. | ||
|
||
**Parameters** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noting that this output is a bit different than we see for data stores in the existing docs, see:
At some point, we should consolidate for consistency.
This PR has taught me two things:
Learning about how to improve the tool was the original goal of this PR, so I guess we could merge it, backport these lesson to the tool and use another package as the next testbed. What do you think @gziolo ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this looks great. I wasn't sure if you want to merge it as is or iterate further. Thanks for working on this PR. It should be super helpful for those who want to write e2e tests 👍
BTW, I moved the WP logo to the bottom of the file for consistency and to make it less exposed :)
Oh, I missed that, thanks for taking care of it! |
Testbed for #13329
The API section within the README of this package has been generated automatically with a new package that's in the works.