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

Improve AppVeyor Configuration #1792

Merged
merged 1 commit into from
Jun 2, 2020
Merged

Improve AppVeyor Configuration #1792

merged 1 commit into from
Jun 2, 2020

Conversation

manuth
Copy link
Contributor

@manuth manuth commented May 31, 2020

Adding support for WSL to AppVeyor caused the appveyor.yml file to look quite complicated.
If applied, this PR will simplify the appveyor.yml-file by temporarily adding a Cmdlet for invoking commands either using . (source) or wsl depending on the current configuration.

This allows us to use the same commands for both WSL and Native, leaving only install-commands as separate sections.

Further Explanation

The GetCI.psm1-file contains a PowerShell function called Get-CICommand. This takes a string containing a command (such as npm install, sudo npm install -g rimraf) and converts it for the appropriate platform:

  • For WSL, a wsl is prepended
    Example: sudo npm i -g rimraf => wsl sudo npm i -g rimraf
  • For Windows, if the first argument is sudo, it is removed as the sudo command doesn't exist on windows and all commands on AppVeyor are executed as admin anyways.
    Example: sudo npm i -g rimraf => npm i -g rimraf

Additionally the ci.cmd script is added to the PATH-variable.
The ci command accepts arguments representing a command (such as npm install, git reset etc.), converts it for the appropriate platform using Get-CICommand (mentioned before) and executes the resulting command using cmd /c (a command similar to bash -c)

Related to #1786

@manuth manuth changed the title Improve Improve AppVeyor Configuration May 31, 2020
scripts/InvokeCI.psm1 Outdated Show resolved Hide resolved
appveyor.yml Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented May 31, 2020

Coverage Status

Coverage remained the same at 97.739% when pulling e1ed323 on manuth:wsl-fix into 9dfef28 on benmosher:master.

@manuth manuth marked this pull request as draft June 1, 2020 02:44
@manuth
Copy link
Contributor Author

manuth commented Jun 1, 2020

I'm almost done, only some command-line outputs during FOR commands are bugging me... hope my next commit will fix this.

@manuth manuth marked this pull request as ready for review June 1, 2020 15:33
@manuth
Copy link
Contributor Author

manuth commented Jun 1, 2020

@ljharb alright, I'm done 😄
Could you please force push this PR back to 0d2fa00?
My most recent commit didn't change anything, so it's not necessary.

@manuth
Copy link
Contributor Author

manuth commented Jun 2, 2020

@ljharb alright, I'm done now 😄
Is it actually allowed to create a PR for adding VSCode workspace files (such as debug config and stuff)?

@ljharb
Copy link
Member

ljharb commented Jun 2, 2020

@manuth no; files related to your personal IDE should go in your personal global gitconfig, instead of adding them to every repo you touch :-)

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Thanks!

@ljharb ljharb merged commit e1ed323 into import-js:master Jun 2, 2020
@manuth manuth deleted the wsl-fix branch June 4, 2020 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants