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

FIX prepare script to delete tests & snapshots etc from dist #7358

Merged
merged 2 commits into from
Jul 9, 2019
Merged

FIX prepare script to delete tests & snapshots etc from dist #7358

merged 2 commits into from
Jul 9, 2019

Conversation

ndelangen
Copy link
Member

@ndelangen ndelangen commented Jul 9, 2019 •
edited
Loading

Issue:

  • We're shipping snapshots & tests in our dist packages
  • snapshots in the dist may be detected by jest and flagged for deletion. These can be impossible to resolve because they are ignored by source control.

What I did

FIX prepare script so it deletes the files copied by babel --copy-files arg, but should have been copied at all

@ndelangen ndelangen added maintenance User-facing maintenance tasks patch:yes Bugfix & documentation PR that need to be picked to main branch labels Jul 9, 2019
@ndelangen ndelangen added this to the 5.1.x milestone Jul 9, 2019
@ndelangen ndelangen requested review from Hypnosphi and igor-dv July 9, 2019 11:29
@ndelangen ndelangen self-assigned this Jul 9, 2019
@vercel
Copy link

vercel bot commented Jul 9, 2019 •
edited
Loading

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://monorepo-git-fix-prepapre-script.storybook.now.sh

shilman
shilman approved these changes Jul 9, 2019
View reviewed changes
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

AFAICT this deletes stories from the distribution. I think we should include stories so that they can be used by other storybooks, at least in lib/components.

shilman
shilman requested changes Jul 9, 2019
View reviewed changes
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Sorry previous comment was supposed to be a change request

@ndelangen
Copy link
Member Author

@shilman Something to discuss long term, I think, we're publishing our full-source with stories & tests in the package already.

I personally think dist should be what consumers need.

@ndelangen ndelangen merged commit fe320c3 into next Jul 9, 2019
@ndelangen ndelangen deleted the fix/prepapre-script branch July 9, 2019 15:50
@shilman shilman added the patch:done Patch/release PRs already cherry-picked to main/release branch label Jul 31, 2019
shilman pushed a commit that referenced this pull request Jul 31, 2019
@ndelangen @shilman
FIX prepare script to delete tests & snapshots etc from dist (#7358)
40f2980 
FIX prepare script to delete tests & snapshots etc from dist
@snyk-bot snyk-bot mentioned this pull request Aug 19, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shilman shilman shilman requested changes

@Hypnosphi Hypnosphi Awaiting requested review from Hypnosphi

@igor-dv igor-dv Awaiting requested review from igor-dv

Assignees

@ndelangen ndelangen

Labels
maintenance User-facing maintenance tasks patch:done Patch/release PRs already cherry-picked to main/release branch patch:yes Bugfix & documentation PR that need to be picked to main branch
Projects
None yet
Milestone
5.1.x
Development

Successfully merging this pull request may close these issues.
2 participants
@ndelangen @shilman