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

Do not run yarn build during postinstall hook #265

Merged
merged 2 commits into from May 22, 2019
Merged

Do not run yarn build during postinstall hook #265

merged 2 commits into from May 22, 2019

Conversation

ranyitz
Copy link
Contributor

@ranyitz ranyitz commented May 22, 2019

Maybe I'm missing something, but running build during postInstall doesn't seem to be the right way to go.

This PR moves it into pretest instead.

Fixes #263

@ranyitz ranyitz changed the title Remove postinstall Do not run yarn build during postinstall hook May 22, 2019
Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

Not what I want, but merging to fix the issue

@SimenB SimenB merged commit 9a7d7f0 into jest-community:master May 22, 2019
@ranyitz ranyitz deleted the patch-1 branch May 22, 2019 15:11
@SimenB
Copy link
Member

SimenB commented May 22, 2019

Thanks, and sorry about this!

I don't wanna build before every test run, but I'll figure something out :)

@swissspidy
Copy link

I thought that's what 9f40d2c should do?

@SimenB
Copy link
Member

SimenB commented May 22, 2019

I don't want to build before every test run, since it'll slow things down.

I'll probably add build to ci, and throw explicit error from eslintrc. But I'm currently in line to board a plane to Paris for react-europe, so this seems like a great quickfix in the meantime :)

@swissspidy
Copy link

Oh, sorry, I misread. Thanks for clarification! Enjoy the conference 🎉

@ranyitz
Copy link
Contributor Author

ranyitz commented May 22, 2019

@SimenB I agree that this is not a good solution for the long term. Thanks for merging :)

@SimenB
Copy link
Member

SimenB commented May 22, 2019

🎉 This PR is included in version 22.6.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@SimenB
Copy link
Member

SimenB commented May 22, 2019

I think using prepare is the correct lifecycle hook, right?

@ranyitz
Copy link
Contributor Author

ranyitz commented May 22, 2019

@SimenB It may be, but it still doesn't solve the local development problem.

I think we should go with adding a yarn build step in our travis configuration (for CI flow)

And for local development, we should create a watch script that runs babel/tsc on every change. prepare would run the build locally after the first install but it won't be much fun running yarn build after each change. (while running jest --watch on a different terminal)

What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade from 22.5.1 to 22.6.0 fails with error /bin/sh: 1: babel: not found
3 participants