-
Notifications
You must be signed in to change notification settings - Fork 5
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 contents of pkg-ci-scripts
to this composite action
#30
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.
Sorry, I didn't notice this PR before :-(.
Looks good to me, some minor remarks/questions, all optional.
Perhaps you could also review gap-actions/process-coverage#7 (and/or improve upon it, whatever)?
run: | | ||
# build some packages; default is to build 'io' and 'profiling', in order to | ||
# generate coverage results. | ||
# Handle GAP_PKGS_TO_BUILD default value | ||
if [ "${{inputs.GAP_PKGS_TO_BUILD}}" = "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.
I know this is old code so not in scope for this PR, but perhaps you know: is the value "default" really ever used? I thought that not specifying it would set it to the value we specified as default, i.e., io profiling
?
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 don't really know, the whole "default" thing was @ssiccha's idea and work. I never use it, anyway.
run: | | ||
# build some packages; default is to build 'io' and 'profiling', in order to | ||
# generate coverage results. |
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.
See also issue #26 (but of course: one step at a time!)
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 agree with #26!
0409079
to
28c1e0c
Compare
This is the beginning of work to address #25.
So far, I have copied the contents of
pkg-ci-scripts/build_gap.sh
into the action, but I have kept cloning thepkg-ci-scripts
repo, because it is still used by the other actions.This could be merged, modulo suggestions from reviews, or it could wait until the other actions are similarly updated.