Add support for macOS platform (default: false) #108
Conversation
shahmishal
force-pushed
the
macos-support
branch
2 times, most recently
from
a9bf7ec to
f31ba17
Compare
ahoppen
left a comment
ahoppen
left a comment
There was a problem hiding this comment.
Wohooooooo. Some comments below but nothing that would block this.
| swift build | ||
| enable_windows_docker: false | ||
|
|
||
| tests_macos: |
There was a problem hiding this comment.
Should this be part of test_with_docker? I would imagine that repos usually define one job that tests all the platforms. I would maybe rename test_with_docker to Test in that case and then we just have test_without_docker left to test Windows without Docker (and could probably rename to test_windows_without_docker.
| macos_versions: | ||
| type: string | ||
| description: "macOS version list (JSON)" | ||
| default: "[\"sequoia\"]" |
There was a problem hiding this comment.
I personally would have probably chosen the release version number (eg. 15 for Sequioa) instead of the marketing name but don’t have strong opinions here. Also just checking: We don’t want to add minor-version fidelity here (eg. the ability to select 15.4 instead of 15.0 or something like that)?
Finally, I think this assumes that we also have a self-hosted runner for this arch + macOS version combination, right? Maybe worth pointing that out in a comment, eg. something like This requires a self-hosted macOS runner with this version to be set up.
If we don’t have near-term plans to support multiple OSs and arches, I would probably remove the option to not give users a false sense that they can test all macOS + arch + Xcode versions.
| macos_archs: | ||
| type: string | ||
| description: "macOS arch list (JSON)" | ||
| default: "[\"ARM64\"]" |
There was a problem hiding this comment.
Same comment as with the macOS version, I would add a comment like: This requires a self-hosted macOS runner with this architecture to be set up.
No description provided.