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

Use yarn compile for start scripts #81840

Merged
merged 1 commit into from Oct 7, 2019

Conversation

@kondratyev-nv
Copy link
Contributor

kondratyev-nv commented Oct 2, 2019

Building and running Code by steps in contributing guide causes FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory error on compiling step. However, compiling with npm run compile works fine. This is because the compile script from package.json has max_old_space_size argument set.

"compile": "gulp compile --max_old_space_size=4095"

This PR adds the same argument to the compile step in scripts.

Copy link
Member

Tyriar left a comment

@joaomoreno any reason we're not running npm run compile in these scripts?

@kondratyev-nv

This comment has been minimized.

Copy link
Contributor Author

kondratyev-nv commented Oct 2, 2019

@Tyriar This is a very helpful comment, thank you! 👍 I also thought about it but did not find any direct usages of npm in these scripts. So I figured that there was some reasoning behind it (maybe the usage of yarn) and just adding the argument was the right way to do it.

@joaomoreno

This comment has been minimized.

Copy link
Member

joaomoreno commented Oct 3, 2019

Just never been adopted. I suggest to use that instead. 👍

@kondratyev-nv kondratyev-nv force-pushed the kondratyev-nv:master branch from 6638882 to db74227 Oct 3, 2019
@kondratyev-nv kondratyev-nv changed the title Set max_old_space_size for start scripts Use npm compile for start scripts Oct 3, 2019
@kondratyev-nv

This comment has been minimized.

Copy link
Contributor Author

kondratyev-nv commented Oct 3, 2019

@Tyriar @joaomoreno I've fixed it, can you have a look?

@joaomoreno

This comment has been minimized.

Copy link
Member

joaomoreno commented Oct 3, 2019

Actually since we are yarn users, do you mind replacing npm run with simply yarn? Thanks.

@kondratyev-nv kondratyev-nv force-pushed the kondratyev-nv:master branch from db74227 to a7f8482 Oct 3, 2019
@kondratyev-nv kondratyev-nv force-pushed the kondratyev-nv:master branch from a7f8482 to 4f6e116 Oct 3, 2019
@kondratyev-nv kondratyev-nv changed the title Use npm compile for start scripts Use yarn compile for start scripts Oct 3, 2019
@Tyriar Tyriar added this to the October 2019 milestone Oct 3, 2019
@Tyriar Tyriar self-assigned this Oct 3, 2019
@Tyriar
Tyriar approved these changes Oct 3, 2019
@joaomoreno joaomoreno merged commit d9935e0 into microsoft:master Oct 7, 2019
2 checks passed
2 checks passed
VS Code #20191003.23 succeeded
Details
license/cla All CLA requirements met.
Details
@joaomoreno

This comment has been minimized.

Copy link
Member

joaomoreno commented Oct 7, 2019

Thanks! 🎆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.