Skip to content
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

Merged
merged 1 commit into from
Nov 23, 2021

Conversation

wilfwilson
Copy link
Contributor

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 the pkg-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.

Copy link
Member

@fingolfin fingolfin left a 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)?

action.yml Show resolved Hide resolved
action.yml Outdated Show resolved Hide resolved
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" ];
Copy link
Member

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?

Copy link
Contributor Author

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.
Copy link
Member

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!)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with #26!

@fingolfin fingolfin merged commit 900f2fb into gap-actions:master Nov 23, 2021
@wilfwilson wilfwilson deleted the add-pkg-ci-content branch November 23, 2021 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants