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

Add win32 platform support. #4

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

yilinwei
Copy link

This adds Windows support to the mill action, using the --no-server
option since it fails on the Github CI with a similar error message
to com-lihaoyi/mill#675.

This adds Windows support to the mill action, using the `--no-server`
option since it fails on the Github CI with a similar error message
to com-lihaoyi/mill#675.
Comment on lines +10 to +20
strategy:
matrix:
include:
- name: Windows
os: windows-latest
mill: 'mill --no-server'
- name: Linux
os: ubuntu-latest
mill: 'mill'
name: ${{ matrix.name }}
runs-on: ${{ matrix.os }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it could be good to check the action runs for both Windows and Linux images, but mill --no-server shouldn't be required now (see com-lihaoyi/mill#781 (comment))

Suggested change
strategy:
matrix:
include:
- name: Windows
os: windows-latest
mill: 'mill --no-server'
- name: Linux
os: ubuntu-latest
mill: 'mill'
name: ${{ matrix.name }}
runs-on: ${{ matrix.os }}
strategy:
matrix:
include:
- name: Windows
os: windows-latest
- name: Linux
os: ubuntu-latest
name: ${{ matrix.name }}
runs-on: ${{ matrix.os }}

Copy link
Author

Choose a reason for hiding this comment

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

Sure that sounds good. I don't think this will be merged anytime soon though - this PR has been outstanding for years.

steps:
- uses: actions/checkout@v1
- uses: ./
name: "Set up Mill"
with:
mill-version: 0.8.0
- name: "Run Mill"
run: mill version
run: ${{ matrix.mill }} version
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
run: ${{ matrix.mill }} version
run: mill version

see above comment

@james-s-w-clark
Copy link
Contributor

@jodersky what do you think of checking it works for Ubuntu/Mac/Windows? Even if people use mill to make jars that are cross platform, they might have a preference for doing it on a given OS in GitHub Actions.

We'd need to add Mac to the OS matrix to finish that.

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