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

Telemetry: Strip out preset from addon name #18442

Merged
merged 1 commit into from Jun 11, 2022

Conversation

yannbf
Copy link
Member

@yannbf yannbf commented Jun 9, 2022

Issue: N/A

What I did

Take into account that some addons are registered with 'addon-name/preset' and strip that out in the metadata.

Also move getStorybookMetadata method to the bottom so it's used after the definition of computeStorybookMetadata, as eslint has warned:

error  'computeStorybookMetadata' was used before it was defined  no-use-before-define

How to test

  • Is this testable with Jest or Chromatic screenshots?
  • Does this need a new example in the kitchen sink apps?
  • Does this need an update to the documentation?

If your answer is yes to any of these, please make sure to include it in your PR.

also move getStorybookMetadata method to the bottom so it's used after the definition
of computeStorybookMetadata
@yannbf yannbf added bug patch Bugfix & documentation PR that need to be picked to master branch telemetry labels Jun 9, 2022
@nx-cloud
Copy link

nx-cloud bot commented Jun 9, 2022

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 56694e9. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


Successfully ran 1 target

Sent with 💌 from NxCloud.

@@ -204,3 +188,19 @@ export const computeStorybookMetadata = async ({
hasStorybookEslint,
};
};

let cachedMetadata: StorybookMetadata;
Copy link
Member Author

@yannbf yannbf Jun 9, 2022

Choose a reason for hiding this comment

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

no changes here, just moved the code down

@codecov
Copy link

codecov bot commented Jun 9, 2022

Codecov Report

Merging #18442 (56694e9) into next (08351b7) will not change coverage.
The diff coverage is 22.22%.

@@           Coverage Diff           @@
##             next   #18442   +/-   ##
=======================================
  Coverage   32.01%   32.01%           
=======================================
  Files         975      975           
  Lines       19234    19234           
  Branches     4033     4033           
=======================================
  Hits         6158     6158           
- Misses      12512    12513    +1     
+ Partials      564      563    -1     
Impacted Files Coverage Δ
lib/telemetry/src/storybook-metadata.ts 77.61% <22.22%> (ø)
addons/a11y/src/components/A11YPanel.tsx 84.09% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 08351b7...56694e9. Read the comment docs.

@yannbf yannbf requested a review from shilman Jun 10, 2022
Copy link
Member

@shilman shilman left a comment

LGTM

@shilman shilman changed the title Telemetry: strip out preset from addon name Telemetry: Strip out preset from addon name Jun 11, 2022
@shilman shilman closed this Jun 11, 2022
@shilman shilman reopened this Jun 11, 2022
@shilman shilman merged commit 58975c5 into next Jun 11, 2022
56 of 62 checks passed
@shilman shilman deleted the fix/telemetry-strip-out-preset branch Jun 11, 2022
@shilman shilman added the picked Patch/release PRs cherry-picked to master/release branch label Jun 12, 2022
shilman added a commit that referenced this issue Jun 12, 2022
…eset

Telemetry: Strip out preset from addon name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug patch Bugfix & documentation PR that need to be picked to master branch picked Patch/release PRs cherry-picked to master/release branch telemetry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants