-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
chore: run windows build on Travis CI #7211
Conversation
d410dc7
to
e1443fb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥🔥🔥
Changleog maybe? |
You can also remove appveyor bits from |
Great call! |
@Daniel15 do you need to remove Jest from your projects or will it be done automatically after we merge this PR? (The builds still trigger even though we removed config) |
Might need a repo admin to remove the webhook from settings? |
|
||
if (global.jasmine && process.env.APPVEYOR_API_URL) { | ||
// Running on AppVeyor, add the custom reporter. | ||
jasmine.getEnv().addReporter(new jasmineReporters.AppVeyorReporter()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, would be nice if they at least support junit reports...
nvs and nvm doesnt agree on naming convention, seemingly
I'm not actually sure. I think an admin of this repo needs to remove the webhook that pings AppVeyor whenever a new PR is opened / commit is made. I'm not an admin so I can't do that. |
5e3f5e1
to
22c89d6
Compare
Hmmm, python is supposed to be there... https://docs.travis-ci.com/user/reference/windows/#pre-installed-packages EDIT: nodejs/node-gyp#1568 |
This is unfortunately blocked for now on nodejs/node-gyp#1568 (not node's fault, but easier to link there than the community thing travis uses) |
@Daniel15 while this PR is blocked, just one last question (sorry for pinging twice 😅) – is it possible for you to automatically cancel previous builds on AppVeyor so they don't queue up unnecessarily? This is our main source of pain right now when testing on Windows. Based on their docs it's possible to set "Rolling builds" from settings UI: https://www.appveyor.com/docs/build-configuration/#rolling-builds |
@thymikee No worries. I just enabled the "rolling builds" option. Feel free to ping me again if there's anything else you'd like me to change. |
4 similar comments
@thymikee No worries. I just enabled the "rolling builds" option. Feel free to ping me again if there's anything else you'd like me to change. |
@thymikee No worries. I just enabled the "rolling builds" option. Feel free to ping me again if there's anything else you'd like me to change. |
@thymikee No worries. I just enabled the "rolling builds" option. Feel free to ping me again if there's anything else you'd like me to change. |
@thymikee No worries. I just enabled the "rolling builds" option. Feel free to ping me again if there's anything else you'd like me to change. |
Codecov Report
@@ Coverage Diff @@
## master #7211 +/- ##
==========================================
- Coverage 67.25% 66.49% -0.77%
==========================================
Files 248 237 -11
Lines 9641 9302 -339
Branches 3 4 +1
==========================================
- Hits 6484 6185 -299
+ Misses 3156 3116 -40
Partials 1 1
Continue to review full report at Codecov.
|
Ok, managed to install python and get it added to EDIT: Also, the windows build doesn't seem to exit properly even though our script completes. It just hangs |
Should not be hanging after the last commit, but we are still missing colors... What about using azure pipelines? Like prettier/prettier#5410 |
Azure Pipelines has support for junit test reporting (as you might've seen prettier use). You can also build on Mac and Linux in addition to Windows, so you could use Azure Pipelines to test OS's and Circle for versions of Node. That said, if you're inclined to move everything to one CI service, Azure Pipelines can test across Node versions too, and we'd be happy to give you additional concurrency. Let me know if you have any questions or suggestions. I'm a Program Manager on Azure Pipelines and happy to help! |
We've been working on moving the Yarn build to Azure Pipelines too :) Seems like a good service. |
I'm +1 for moving to azure instead 😸 |
Yeah, my vote is for Azure pipelines as well. We don't run Travis on master since colors are messed up, which means we'd not be running windows on master, which I don't think is acceptable. @kaylangan What's the easiest way forward? I'm not an admin on this repo, is that needed?
That sounds perfect, at least to start with 🙂 |
@SimenB I've created #7556 that will add initial support for Azure Pipelines. You will need repo permissions to set up the Azure Pipelines GitHub app. |
let's go for #7556 |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
🤞
Appveyor is super slow, and on a private account, so we can't cancel redundant builds there. And going from 3 to 2 CIs is only positive.
We might consider adding osx as well, so we have Circle for testing different versions of Node, and Travis to test different OSes.
https://blog.travis-ci.com/2018-10-11-windows-early-release
Test plan
Green!