-
Notifications
You must be signed in to change notification settings - Fork 244
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
fix: add missing config-file option to the action #883
fix: add missing config-file option to the action #883
Conversation
action.yml
Outdated
config-file: | ||
description: 'File containing the config, defaults to {package}/pyproject.toml' | ||
required: false | ||
default: '{package}/pyproject.toml' |
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.
It would be nice if we didn't have to specify the default here too, from a DRY perspective.
ahhhh, I think this default might present an issue, actually. If a config file is specified, then cibuildwheel requires it to exist. If it's not set, then the Options class will use pyproject.toml, if it's present. So this wouldn't work for projects without a pyproject.toml.
Maybe we'd have to use some bash to remove the argument if it's not set in the action? Or we could change Options to interpret config_file=""
as 'not set'.
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.
Ahh, yes, I thought of that before, and then forgot about it.
I was thinking --config-file=""
would be an option to turn off config file discovery completely; if you want to ignore the config in pyproject.toml
, this would be a handy shortcut, possibly?
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.
That would then also provide different behavior for this option, too, and we haven't supported empty strings here in the past - I've changed this to config_file=""
as 'not set'. Someone who wanted to override the pyproject.toml and didn't want a new toml file can just point this at an empty file, I suppose.
220918c
to
816be41
Compare
816be41
to
4b7e0be
Compare
Co-authored-by: Joe Rickerby <joerick@mac.com>
This is missing, but might be handy if users want to use config files other than pyproject.toml.