Update appveyor to use yarn instead of npm and auto fix windows line … #45
Conversation
d4r5c0d3
commented
Jun 21, 2017
d4r5c0d3
commented
- Added fix to force windows line endings
- Added yarn to cache
- Replaced npm with yarn for install
- Removed npm version check
appveyor.yml
Outdated
| - "%LOCALAPPDATA%\\Yarn" | ||
|
|
||
| init: | ||
| - git config --global core.autocrlf true |
There was a problem hiding this comment.
-1 for that. We need to make sure that encore works without altering the source code (because normal usage will not do such conversion)
There was a problem hiding this comment.
@stof Could you explain what you mean?
I got this from the yarn website: https://yarnpkg.com/lang/en/docs/install-ci/#appveyor-tab
Looking at other projects they use the same line
There was a problem hiding this comment.
The doc you linked does not have this line
There was a problem hiding this comment.
@stof You could have been a bit more clear as to what you were talking about.
Your feedback leaves allot of room for misunderstanding.
This is not best practices considering windows.There are cases where you will run into trouble with this. Since I don't think we will have those here I'll remove autocrlf line endings on your request.
There was a problem hiding this comment.
Well, my comment is on the exact line to which it applies.
And converting the code to CRLF on checkout always created bug in my experience (I'm not using Windows anymore for development, but I did it during years), because it means you are not running the source code of the project but a different one. The meaningful value IMO is input, not true (but this is irrelevant on CI as it will never commit anything). The only valid use case for this setting AFAIK is being able to open the code in Notepad (which is the only text editor I know of which does not support Unix line endings, but I would never use it to write code anymore).
Thus, it is even worse for CI: people installing Encore on Windows will not get the line endings converted (as npm/yarn will not use git for that, but download a tarball, and so won't be affected by your git config), meaning that converting line endings on CI would mean that the CI does not test the code that users will use.
|
Thanks for all your help with this @DaRamirezSoto! The Windows support should be fixed/merged soon - I'm very happy with how it's all setup - I want Windows to work great :). |
…dows line … (DaRamirezSoto) This PR was squashed before being merged into the master branch (closes #45). Discussion ---------- Update appveyor to use yarn instead of npm and auto fix windows line … - Added fix to force windows line endings - Added yarn to cache - Replaced npm with yarn for install - Removed npm version check Commits ------- 72fb23f Remove autocrlf from AppVeyor f388286 Update appveyor to use yarn instead of npm and auto fix windows line endings
|
@weaverryan No problem, I like to help more but for now I cannot because of current projects. |
