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

Switch to the only prepublish script #903

Merged
merged 5 commits into from
Apr 24, 2017
Merged

Switch to the only prepublish script #903

merged 5 commits into from
Apr 24, 2017

Conversation

usulpro
Copy link
Member

@usulpro usulpro commented Apr 16, 2017

Issue: All packages use different versions of prepublish scripts. Plus some of them shell scripts which have bad compatibility with windows

What I did

  • Added the only instance of prepublish script to
    ./scripts/prepublish.js
    it's the shell js version wich allready was used in storybook-ui

  • Changed in scripts of each package.json to
    "prepublish": "node ../../scripts/prepublish.js"

  • Removed previos versions of prepublish scripts from packages

How to test

  1. clone this repo
  2. run yarn
    it should install all dependencies without any errors and buid dist's for all packages

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.

Great change! See minor comment above.

const path = require('path');
const shell = require('shelljs');
const chalk = require('chalk');
const babel = ['node_modules', '.bin', 'babel'].join(path.sep);
Copy link
Member

Choose a reason for hiding this comment

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

Use path.join?

Copy link
Member Author

Choose a reason for hiding this comment

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

sure!

@usulpro
Copy link
Member Author

usulpro commented Apr 18, 2017

Thanks, @shilman!
I added --copy-files flag as well

@usulpro usulpro removed the request for review from ndelangen April 21, 2017 00:52
@usulpro
Copy link
Member Author

usulpro commented Apr 21, 2017

So if nobody objects I will merge this?

@ndelangen
Copy link
Member

@usulpro Are you working on getting this merged?

@usulpro
Copy link
Member Author

usulpro commented Apr 23, 2017

Yes @ndelangen I'll resolve it

usulpro added 5 commits April 24, 2017 17:33
use ./scripts/prepublish.js as a prepublish script for all packages
stories -> stories/ to ignore only folders
added test.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants