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

fix: [M3-8904] - Fix Storybook docgen #11264

Merged

Conversation

hkhalil-akamai
Copy link
Contributor

Description 📝

Fixes docgen in Storybook, which is currently broken when launching through yarn storybook. Docgen converts TSDoc comments into documentation for props and components.

Changes 🔄

Preview 📷

Before After
Screenshot 2024-11-14 at 3 50 41 PM Screenshot 2024-11-14 at 3 48 54 PM

How to test 🧪

  1. Start Storybook via yarn storybook
  2. Verify component stories have documentation generated from TSDoc comments (see screenshots).

@hkhalil-akamai hkhalil-akamai self-assigned this Nov 14, 2024
@hkhalil-akamai hkhalil-akamai requested a review from a team as a code owner November 14, 2024 20:54
@hkhalil-akamai hkhalil-akamai requested review from dwiley-akamai and abailly-akamai and removed request for a team November 14, 2024 20:54
@hkhalil-akamai hkhalil-akamai added the Add'tl Approval Needed Waiting on another approval! label Nov 14, 2024
Copy link

github-actions bot commented Nov 14, 2024

Coverage Report:
Base Coverage: 87.34%
Current Coverage: 87.34%

@linode-gh-bot
Copy link
Collaborator

Cloud Manager UI test results

🎉 445 passing tests on test run #2 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
0 Failing445 Passing2 Skipped80m 17s

Copy link
Contributor

@abailly-akamai abailly-akamai left a comment

Choose a reason for hiding this comment

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

I can't even load storybook anymore but that's unrelated to this change. The cache is messed up (we gonna need to configure optimizeDeps).

Changes look good otherwise.

Have you considered keeping an include in reactDocgenTypescriptOptions to only target the packages we care about (manager and UI). Not sure about performance benefits but worth mentioning

@jaalah-akamai jaalah-akamai removed the Add'tl Approval Needed Waiting on another approval! label Nov 15, 2024
@jaalah-akamai jaalah-akamai added the Approved Multiple approvals and ready to merge! label Nov 15, 2024
Copy link
Contributor

@dwiley-akamai dwiley-akamai left a comment

Choose a reason for hiding this comment

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

All checks pass ✅
More comprehensive documentation is displayed in Storybook ✅

@hkhalil-akamai hkhalil-akamai merged commit 8492246 into linode:develop Nov 15, 2024
23 checks passed
@hkhalil-akamai hkhalil-akamai deleted the M3-8904-fix-storybook-docgen branch November 15, 2024 19:55
@hkhalil-akamai
Copy link
Contributor Author

Have you considered keeping an include in reactDocgenTypescriptOptions to only target the packages we care about (manager and UI). Not sure about performance benefits but worth mentioning

On the fence considering the marginal gain from added configuration.

Copy link

cypress bot commented Nov 15, 2024

Cloud Manager E2E    Run #6831

Run Properties:  status check passed Passed #6831  •  git commit 8492246e4c: fix: [M3-8904] - Fix Storybook docgen (#11264)
Project Cloud Manager E2E
Branch Review develop
Run status status check passed Passed #6831
Run duration 27m 46s
Commit git commit 8492246e4c: fix: [M3-8904] - Fix Storybook docgen (#11264)
Committer Hussain Khalil
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 2
Tests that did not run due to a developer annotating a test with .skip  Pending 2
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 452
View all changes introduced in this branch ↗︎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge! Storybook
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants