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

docs: adds generated reference #2269

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lukegalbraithrussell
Copy link
Contributor

@lukegalbraithrussell lukegalbraithrussell commented Sep 20, 2024

Summary

This adds a generated reference. Leaving it "Generated Reference" to compare with the current "Reference" as feedback is gathered. Let me know what you think! Just throwing it up here before I'm off for a bit next week

The generated files are not checked into git because there are hundreds. They'll show up on the site with npm run start or npm run build though! Grab this branch and run it locally on your machine to view the docs.

boltjs-generated-reference.mov

Requirements (place an x in each [ ])

Copy link

codecov bot commented Sep 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.78%. Comparing base (e82f6e8) to head (027b11e).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2269   +/-   ##
=======================================
  Coverage   87.78%   87.78%           
=======================================
  Files          19       19           
  Lines        1646     1646           
  Branches      464      464           
=======================================
  Hits         1445     1445           
  Misses        194      194           
  Partials        7        7           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zimeg zimeg added the docs M-T: Documentation work only label Sep 20, 2024
Copy link
Member

@zimeg zimeg left a comment

Choose a reason for hiding this comment

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

📚 😭 It's so great to find all of these comments in website format. I'm pretty dang hyped for this!

During browsing I noticed a few strange things without a super clear cause to me. These aren't blockers IMO but it might be nice to at least check out before more review 🙏

🪓 Stricken and deprecated reference

This might ackchyually be caused from JSdoc comments and not the changes here, so feel free to ignore in this PR. I do think it's worth reviewing all of these eventually, but maybe after major changes settle. Anyways, this page me by surprise:

actionable

http://localhost:3000/bolt-js/generated-reference/interfaces/Actionable

It makes sense that we include deprecated reference IMO, but I do think this is just a strange case I'm calling out...

🔍 References to libraries

Typing details from other packages and modules are included in reference which at first seemed noisy, but I do think this is useful for completeness (which IMHO is what the ideal reference has).

definitions

We might want to consider the Node version we're using for some of this reference though. AFAIK the typings between Node 20 (the current build) and Node 22 can change and are often more verbose or improved, but this might cause conflicts with built-in types:

http://localhost:3000/bolt-js/generated-reference/classes/AppInitializationError#returns

🚂 Sidebar overflows

This seems to be strangeness with Docusaurus, but I find when i CTRL+F characters beyond the shown sidebar, strange wrapping can happen (although this is no blocker at all IMO):

sidebar

.stylelintrc.json
content/generated-reference
Copy link
Member

Choose a reason for hiding this comment

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

Super neat that we can generate this just for the final build!

Copy link
Member

Choose a reason for hiding this comment

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

Nit: Could these lines be indented with 2 spaces 👾

Copy link
Contributor

@filmaj filmaj left a comment

Choose a reason for hiding this comment

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

Looking great!

+1 to everything @zimeg mentioned.

Also: could we strip out the JSDoc-specific tags, like @description? Feel like that's redundant.

@seratch seratch removed their request for review September 25, 2024 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs M-T: Documentation work only
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants