-
Notifications
You must be signed in to change notification settings - Fork 27
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
Provide filters for ci builds #162
Conversation
.github/workflows/build.yml
Outdated
python_version = ["3.7", "3.8", "3.9"] | ||
build = { "macos": ["macos-10.15"], "linux": ["ubuntu-latest"], "windows": ["windows-2016"] } | ||
test = { "macos": ["macos-10.15", "macos-11"], "linux": ["ubuntu-18.04", "ubuntu-20.04"], "windows": ["windows-2016", "windows-2019", "windows-2022"] } | ||
build = [ "macos-10.15", "ubuntu-18.04", "windows-2016" ] |
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.
I fail to see how changing ubuntu-latest into ubuntu-18.04 is related to this PR. Anyway this change is useless.
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.
This is the implementation of:
"By default, built are done on the oldest versions of the oses"
If you build on the latest os, you cannot guarantee that it will work on an oldest one.
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.
Yes we can™, this is why we build with manylinux on Linux
.github/workflows/build.yml
Outdated
needs: | ||
- test-ubuntu | ||
- test-macos | ||
- test-windows |
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.
Why do you suddenly decide that 'needs' should be written on multiple lines? Previous version looks more readable to me
.github/workflows/build.yml
Outdated
build = { "macos": ["macos-10.15"], "linux": ["ubuntu-latest"], "windows": ["windows-2016"] } | ||
test = { "macos": ["macos-10.15", "macos-11"], "linux": ["ubuntu-18.04", "ubuntu-20.04"], "windows": ["windows-2016", "windows-2019", "windows-2022"] } | ||
build = [ "macos-10.15", "ubuntu-18.04", "windows-2016" ] | ||
test = [ "macos-10.15", "macos-11", "ubuntu-18.04", "ubuntu-20.04", "windows-2016", "windows-2019" ] |
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.
why did you remove windows-2022? This is not mentioned in your description
BTW it builds without error.
.github/workflows/release.yml
Outdated
'windows-2016' : to_bool("${{ contains(github.event.head_commit.message, '[ci: windows-2016]') }}"), | ||
'windows-2019' : to_bool("${{ contains(github.event.head_commit.message, '[ci: windows-2019]') }}"), | ||
} | ||
if set(os_filter.keys()) != set(test): |
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.
Your description states that there is only one keyword parsed by release.yml. This is obviously false.
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.
The statement means that only one keyword is active in release
. Ideally the setup could/should be the same for both workflow.
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.
The statement means that only one keyword is active in release.
Which is false. It would be much more obvious if you did not copy everything but only keep the keyword you want to parse.
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.
Okay I stand corrected, my statement is wrong, all keywords are active, but needs.setup is never used. But this is error prone, dead code should be removed.
.github/workflows/build.yml
Outdated
build-docs: | ||
needs: [test-linux, test-macos, test-windows] | ||
build-doc: | ||
needs: [test-ubuntu, test-macos, test-windows] |
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.
you must add setup in needs, docs are currently not built.
By default, the build is done on the oldest version of each OS For Linux, manylinux in a virtual env is used. Switches for build are: - [ci: os-version] will test the specific version of the specified os if the version is ommitted then all default versions will be used - [ci: python-x.y] will build and test only using python version x.y - [ci: skip-doc] will skip the build of the doc release as so far one switch only: - [ci: skip-deploy-test-pypi] will avoid deploying on Test PyPi
Release tested and results visible here: https://github.com/galleon/scikit-decide/actions/runs/1583655111 |
By using specific keywords the user can interact on the ci build and release workflows
For
build
the following keywords are available:For
release
, only one keyword is available: