-
Notifications
You must be signed in to change notification settings - Fork 4
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
Introduce GitHub Action to install project requirements #139
Conversation
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.
All looks fairly straightforward; two suggestions around argument handling that might help avoid surprises in the future.
extra: | ||
description: "Name of the optional dependencies marker; e.g. dev." | ||
required: false | ||
default: "" |
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.
Checking that I'm reading this right - if you don't specify extra as an argument, this manifests as --extra ""
in the final arguments, right? Would it be worth gating the --extra
argument with an if inputs.extra
?
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 default value of extra
within the install_requirement.py
script is an empty string. So, passing --extra
an empty string at the command-line is equivalent to not specifying --extra
at all. With that, we can control specifying --extra
at the command-line but it's effectively duplicating effort the script already handles.
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 said...
python -m install_requirement ${{ inputs.requirements }} \
- --extra "${{ inputs.extra }}" \
+ ${{ inputs.extra && format('--extra {0}', inputs.extra) || '' }} \
--project-root "$(printf -- "%q" "${{ steps.paths.outputs.project-root }}")"
122f70c
to
4f15cd1
Compare
4f15cd1
to
a62ae86
Compare
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.
👍
Changes
pyproject.toml
PR Checklist: