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.

Sign up for GitHub

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

Jump to bottom

SB Metadata Retrieval #585

Merged
merged 30 commits into from
Jun 7, 2022
Merged

SB Metadata Retrieval #585

merged 30 commits into from
Jun 7, 2022

Conversation

thafryer
Copy link
Member

@thafryer thafryer commented May 31, 2022 •
edited

This PR updates how we retrieve the metadata for the Storybook being used in Chromatic.

  1. If the Storybook is being uploaded via -s flag and the project.json file exists (meaning Storybook is v6.5+), we will use the data in that project.json file.
  2. We also now have access to which builder was used.
  3. We updated the way that we retrieve the package.json information.

@linear
Copy link

linear bot commented May 31, 2022

AP-1933 Add builder packages to Chromatic CLI telemetry

(Also fix existing telemetry which currently detects no packages)

@thafryer thafryer changed the title draft: SB Metadata Retrieval SB Metadata Retrieval Jun 2, 2022
tmeasday
tmeasday reviewed Jun 3, 2022
View reviewed changes
Copy link
Member

@tmeasday tmeasday left a comment •
edited

Choose a reason for hiding this comment

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

I like it! Would like @ghengeveld to take a look though.

@thafryer @tmeasday
Fix Typo
92db2b9 
Co-authored-by: Tom Coleman <tom@thesnail.org>
ghengeveld
ghengeveld requested changes Jun 3, 2022
View reviewed changes
Copy link
Member

@ghengeveld ghengeveld left a comment

Choose a reason for hiding this comment

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

Impressive work! I'm happy I didn't have to figure this out myself.
I left a bunch of suggestions for improvements, but my primary feedback is the (lack of) testing of getPrebuiltStorybookMetadata, because it's getting mocked out.

package.json Show resolved Hide resolved
bin-src/main.ts Outdated Show resolved Hide resolved
bin-src/lib/getStorybookInfo.ts Show resolved Hide resolved
bin-src/lib/getPrebuiltStorybookMetadata.ts Outdated Show resolved Hide resolved
bin-src/lib/getPrebuiltStorybookMetadata.ts Outdated Show resolved Hide resolved
bin-src/lib/getStorybookMetadata.ts Outdated Show resolved Hide resolved
bin-src/lib/getStorybookInfo.test.ts Outdated Show resolved Hide resolved
bin-src/lib/getStorybookInfo.test.ts Outdated Show resolved Hide resolved
thafryer and others added 5 commits June 3, 2022 16:26
@thafryer
Add actual project json files to be read
2f4cbb5
@thafryer
Merge branch 'jarel/ap-1933-add-builder-packages-to-chromatic-cli' of…
db5d420 
… github.com:chromaui/chromatic-cli into jarel/ap-1933-add-builder-packages-to-chromatic-cli
@thafryer
make getBuilder more readable
6399c96
@thafryer @ghengeveld
refactor addons map for prebuiltMetadata
c5e17a1 
Co-authored-by: Gert Hengeveld <info@ghengeveld.nl>
@thafryer @ghengeveld
remove if branch for findBuilder
32ba9fd 
Co-authored-by: Gert Hengeveld <info@ghengeveld.nl>
JonathanKolnik
JonathanKolnik approved these changes Jun 6, 2022
View reviewed changes
Copy link
Contributor

@JonathanKolnik JonathanKolnik left a comment

Choose a reason for hiding this comment

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

Seems like you addressed all of Gert's latest comments, so looks good to me! Nice work 😄

@thafryer thafryer merged commit a07b004 into main Jun 7, 2022
@thafryer thafryer deleted the jarel/ap-1933-add-builder-packages-to-chromatic-cli branch June 7, 2022 20:17
@thafryer thafryer mentioned this pull request Jun 7, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ghengeveld ghengeveld ghengeveld approved these changes

@JonathanKolnik JonathanKolnik JonathanKolnik approved these changes

@tmeasday tmeasday Awaiting requested review from tmeasday

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@thafryer @tmeasday @ghengeveld @JonathanKolnik