Add appveyor.yml config file#299
Add appveyor.yml config file#299nfischer merged 1 commit intoshelljs:masterfrom nfischer:appveyor-support
Conversation
|
I will close this if appveyor doesn't get triggered |
|
LGTM, but let's wait to be able to actually enable AppVeyor. |
|
@ariporad yup. I think this shouldn't be merged until appveyor is set up properly and it actually runs on the PR (so we might have to reopen another PR after that and close this one). |
|
You can always rebase this change on the latest from master then force push the branch. That will re-trigger AppVeyor just fine without need to close/re-open this PR. If the branch is up to date with the latest from master you can amend this commit (change the commit message) and force push which will also re-trigger things in AppVeyor. Once the Webhook stuff is triggered that is. |
|
Yeah, that's one way. I also wanted to verify that it works on PRs as well. But since the config file is most likely good as it is, we can probably just merge once the appveyor settings and webhooks are straightened out. |
|
@nfischer: I think we might want to get our tests passing on windows before this gets merged. But it did work! |
|
@ariporad sounds good to me |
|
@ariporad You say it worked, but I don't see an AppVeyor status check on this PR for AppVeyor. |
|
@mtscout6 appveyor should now be running on this PR (I updated an equivalent PR, but forgot to do this one) |
|
Perfect! Glad to see it working! |
|
Yup! And it looks similar to the output @BYK got when running the tests, which is a good sign |
|
@ariporad I'd advocate merging this now instead of waiting for tests to pass. That way the you can see that the tests do fix the tests in the PRs that are fixing them. It's not a "Required" check for merging, though once it goes green then you can make it a required check if you like. That's my opinion though. |
|
LGTM |
This is a redo of #298