scalacenter / scalajs-bundler

https://scalacenter.github.io/scalajs-bundler
Other
237 stars 101 forks source link

Drop AppVeyor #422

Closed ptrdom closed 2 years ago

ptrdom commented 2 years ago

With CI being already migrated to GitHub Actions, keeping AppVeyor seems unnecessary. Less CI scripts to maintain. Since GitHub Actions require approval for first time contributors and AppVeyor did not, maybe it would be good to reconsider loosening this rule for GitHub Actions in this repository.

sjrd commented 2 years ago

The point of the AppVeyor build is to test on Windows. If we remove AppVeyor, we must add a Windows build to the GitHub Actions CI.

ptrdom commented 2 years ago

GitHub has Windows machines available for running Actions, with matrix it is relatively easy to setup, I will add it to this PR.

sjrd commented 2 years ago

The Windows build doesn't seem to test anything. It completed in 1 minute without doing much of anything. By comparison, the Linux build takes more than 15 minutes.

ptrdom commented 2 years ago

Really strange. Posted in community about it - https://github.community/t/windows-runner-not-executing-sbt-commands/250507.

armanbilge commented 2 years ago

Note that olafurpg/setup-scala action is effectively deprecated. It might not be doing the right things.

ptrdom commented 2 years ago

@armanbilge You were right, many thanks! One test does fail, and the runner seems quite slow, but the same test fails on my Windows machine too. AppVeyor tests run on older WIndows Server version, so that must be related. Will look into that next.

ptrdom commented 2 years ago

Looks nasty:

Error: [error] (server / Compile / doc) java.net.URISyntaxException: Illegal character in path at index 53: [https://docs.oracle.com/en/java/javase/11/docs/api/C:\Users\runneradmin\AppData\Local\Coursier\cache\jvm\adopt@1.11.0-11\jmods\java.base/java/lang/Class.html](https://docs.oracle.com/en/java/javase/11/docs/api/C:/Users/runneradmin/AppData/Local/Coursier/cache/jvm/adopt@1.11.0-11/jmods/java.base/java/lang/Class.html)

https://github.com/scalacenter/scalajs-bundler/runs/6439309039?check_suite_focus=true#step:8:694

ptrdom commented 2 years ago

Looks related to https://github.com/scala/bug/issues/11955.

ptrdom commented 2 years ago

@sjrd Does this look mergeable to you?

sjrd commented 2 years ago

LGTM. Could you squash everything in a single commit with a clean commit message, please? Please also explain the change of Scala version in the test.

ptrdom commented 2 years ago

@sjrd Done!