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

Move to yarn #1130

Merged
merged 1 commit into from
Sep 5, 2017
Merged

Move to yarn #1130

merged 1 commit into from
Sep 5, 2017

Conversation

MangelMaxime
Copy link
Member

@alfonsogarciacaro
Copy link
Member

Super fast, thank you! 🏎 🏎 🏎

@alfonsogarciacaro alfonsogarciacaro merged commit e1e369b into master Sep 5, 2017
@ncave
Copy link
Collaborator

ncave commented Sep 7, 2017

Should the yarn package be added as paket dependency? The build is currently failing on Windows with:

System.Exception: Start of process C:\Projects\Fable\packages\Yarnpkg.js\tools\yarn.cmd failed. The system cannot
find the file specified
   at Fake.ProcessHelper.ExecProcessWithLambdas@103-16.Invoke(String message) in D:\code\fake\src\app\FakeLib\Proc

@MangelMaxime
Copy link
Member Author

yarn is recommanded to be installed globally on your system. So it's a manual action.

After yarn is also distributed via npm so we could cheat but i know no one going this way.

@MangelMaxime
Copy link
Member Author

I think I so something about installing yarn via fake I will take a look

@ncave
Copy link
Collaborator

ncave commented Sep 7, 2017

@MangelMaxime yarn is already installed globally, but that's not what the error is about, it seems to be missing a package in the packages folder.

@MangelMaxime
Copy link
Member Author

MangelMaxime commented Sep 7, 2017

I don't understand is why you have yarn under your packages folder.

Because for me your error is it's doesn't find the file C:\Projects\Fable\packages\Yarnpkg.js\tools\yarn.cmd. @ncave Could you please try to clean your packages folder just to check ? The CI are green so it's seems to have something to do with your computer.

@ncave
Copy link
Collaborator

ncave commented Sep 7, 2017

@MangelMaxime That's the first thing I did, it doesn't help. I also tried on two different machines. Yes, looks like Fake is looking for \Fable\packages\Yarnpkg.js\tools\yarn.cmd, is that package there in CI? I see you do some additional pre-install on TravisCI with curl -o- -L https://yarnpkg.com/install.sh, but I don't see anything similar in the appveyor.yml

@alfonsogarciacaro
Copy link
Member

My guess is the FAKE helper first checks if Yarn is installed globally and then tries the packages folder. Yarn is already installed in AppVeyor and this PR also installs it for the Travis image, so that's probably why CI passes.

It'd be good to have Yarn downloaded locally so it's a global requirement less, but I'm not sure how to do it. Apparently this FAKE PR just tries to locate yarn as if it were the Nuget npm package but that doesn't look quite right.

@alfonsogarciacaro
Copy link
Member

alfonsogarciacaro commented Sep 7, 2017

@ncave The FAKE PR checks the path first, I guess this is how it's located in AppVeyor. Do you have yarn in your path?

https://github.com/fsharp/FAKE/pull/1494/files#diff-b7934a24ef87f312fdb430e3e0355861R12

@ncave
Copy link
Collaborator

ncave commented Sep 7, 2017

@alfonsogarciacaro No, I had it installed globally through npm. I'll install it with the windows installer, thanks for the help.

@ncave
Copy link
Collaborator

ncave commented Sep 7, 2017

Works now. Thanks for the help, @MangelMaxime and @alfonsogarciacaro, the problem was located between the keyboard and the chair.

@MangelMaxime
Copy link
Member Author

Nice if you got it working :)

For AppVeyor, if it detect a yarn.lock at the root of the project it will install yarn.
However, it's still not the case of Travis so we need to install it our self.

@alfonsogarciacaro alfonsogarciacaro deleted the yarn_again branch November 30, 2017 20:37
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.

3 participants