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

Refactor install steps into resusable workflow #341

Merged
merged 2 commits into from
Dec 16, 2023

Conversation

pvandyken
Copy link
Contributor

Completing the Actions refactoring suggested by @tkkuehn.

@kaitj, you're welcome to take the flow and incorporate it into your centralized repository, if you wish

Copy link
Contributor

@kaitj kaitj left a comment

Choose a reason for hiding this comment

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

This largely looks good to me with one question. On pushing new changes to an open PR, does it reference github.event.pull_request.id or github.event.after? If the latter, would this mean the setup is rerun everytime there is a new push? (Assuming this references the commit hash.)

And yes, I will end up throwing it in the centralized repo! I've added a modified version of it in the centralized repo, pending a test before merging into main. Had to change it slightly as (I think) it had to be defined as a job rather than a composite action. In any case, I'll ping you over there once I make the PR.

@kaitj
Copy link
Contributor

kaitj commented Sep 26, 2023

The install action is now available on the tagged v0.2.0v0.2.1 in the central repo.

@pvandyken pvandyken requested a review from kaitj December 11, 2023 21:53
@pvandyken pvandyken force-pushed the maint/gh-actions/refactor branch from b54ff34 to ec3e0a9 Compare December 11, 2023 22:05
Copy link
Contributor

@kaitj kaitj left a comment

Choose a reason for hiding this comment

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

This largely looks good to me, but there is the one action path that I am not sure if it exists. Once that is confirmed to exist / updated, then this one is good to go.

.github/workflows/test.yml Outdated Show resolved Hide resolved
id: setup-python
uses: actions/setup-python@v4
- name: Install
uses: khanlab/actions/.github/actions/action-setup_task-installPyProject
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't recall at the moment - wondering if you know what the behaviour is if you don't tag a specific version? I assume it would either use the latest tag or whatever is on main?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, I'll add a tag

@pvandyken pvandyken added maintenance Updates or improvements that do not change functionality of the code blocked Feature currently blocked from being implemented labels Dec 12, 2023
Copy link
Contributor

@kaitj kaitj left a comment

Choose a reason for hiding this comment

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

This one is good to go once the installPyProject tag is updated.

@pvandyken pvandyken force-pushed the maint/gh-actions/refactor branch from 53d732e to 4a3d59c Compare December 13, 2023 21:39
Regex was not matching the entire container path, leading to incorrect
error messages
Project installation uses khanlab/actions

Docker container setup is refactored into a composite rule
@pvandyken pvandyken force-pushed the maint/gh-actions/refactor branch from 4a3d59c to a36eca3 Compare December 14, 2023 16:54
@pvandyken pvandyken removed the blocked Feature currently blocked from being implemented label Dec 14, 2023
@pvandyken pvandyken merged commit 1aefca5 into khanlab:main Dec 16, 2023
28 checks passed
@pvandyken pvandyken deleted the maint/gh-actions/refactor branch December 16, 2023 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Updates or improvements that do not change functionality of the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants