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

(chore) Switch to AppVeyor for Windows testing #1255

Merged
merged 1 commit into from Dec 27, 2019
Merged

(chore) Switch to AppVeyor for Windows testing #1255

merged 1 commit into from Dec 27, 2019

Conversation

XhmikosR
Copy link
Contributor

@XhmikosR XhmikosR commented Dec 27, 2019

@coreyfarrell the only changes you should make in AppVeyor settings are these:

  • Skip branches without appveyor.yml
  • Save build cache in Pull Requests

Notes:

  • we could always install the latest version of Node.js; if you want this let me know
  • I haven't added Node.js 13, should I add it?
  • I test one x86 build and everything else x64. I don't think it matters, so maybe I should just use x64 for all versions?
  • I have added caching (caching the global npm folder) which will be invalidated if any of these files changes: appveyor.yml, package.json, package-lock.json

Copy link
Member

@coreyfarrell coreyfarrell left a comment

Choose a reason for hiding this comment

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

Appears I didn't have appveyor fully enabled, this should now be fixed so next time you do a push github should run it.

README.md Show resolved Hide resolved
appveyor.yml Outdated Show resolved Hide resolved
appveyor.yml Show resolved Hide resolved
@XhmikosR
Copy link
Contributor Author

Alright, I pushed my changes, if it passes I will squash the patches.

So, assuming you made the two changes in settings I mentioned, and you are OK with how this works, you can merge after that.

Forgot to mention I have specified shallow_clone: true which is basically cloning with depth: 1

@coreyfarrell
Copy link
Member

Alright, I pushed my changes, if it passes I will squash the patches.

No need, I use Squash and merge on the github interface even if you already squashed. We use commit logs for changelog generation and this gives us the best results (the PR is added to the first line of the commit).

So, assuming you made the two changes in settings I mentioned, and you are OK with how this works, you can merge after that.

Settings are changed per your suggestion.

Forgot to mention I have specified shallow_clone: true which is basically cloning with depth: 1

This is fine.

@XhmikosR
Copy link
Contributor Author

Some further notes:

No need, I use Squash and merge on the github interface even if you already squashed. We use commit logs for changelog generation and this gives us the best results (the PR is added to the first line of the commit).

I'm still old school :P

  1. Always installing the latest Node.js version results in some slowness, ~1min 30s
  2. The free AppVeyor plan is limited in concurrency unfortunately

I will push the last changes and if everything is green you can merge.

@XhmikosR
Copy link
Contributor Author

OK so it seems the Current alias does not work with always using the latest version, or at least I couldn't make it work. I just used 13 for now.

@XhmikosR
Copy link
Contributor Author

Should I add fast_finish: true?

@coreyfarrell
Copy link
Member

Should I add fast_finish: true?

Yes please

@XhmikosR
Copy link
Contributor Author

One last thing, what is the AppVeyor project URL? it seems https://ci.appveyor.com/project/istanbuljs/nyc isn't right

@XhmikosR
Copy link
Contributor Author

NVM my mind was stuck for a while :P It's https://ci.appveyor.com/project/coreyfarrell/nyc

@coreyfarrell
Copy link
Member

That's weird, I don't understand why it's listed under by username instead of istanbuljs? Did I do something wrong?

@XhmikosR
Copy link
Contributor Author

I remember I had that issue too in the past. Not sure how to solve it. But I updated the badge/link to point to your project for now.

@coreyfarrell
Copy link
Member

Sorry please switch the badge back, I just found the setting so the appveyor URL is now https://ci.appveyor.com/project/istanbuljs/nyc

@XhmikosR XhmikosR marked this pull request as ready for review December 27, 2019 15:21
@XhmikosR
Copy link
Contributor Author

Alright, this is ready. As much as I'd like to always test against the latest Node.js versions, this is significantly slower. On the other hand this will allow you to test say v14 even before AppVeyor adds it to their images.

Also, switch to shields.io for all badges (minus the Slack one)
@coreyfarrell coreyfarrell merged commit 676ca86 into istanbuljs:master Dec 27, 2019
@XhmikosR XhmikosR deleted the appveyor branch December 27, 2019 16:38
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.

None yet

2 participants