-
Notifications
You must be signed in to change notification settings - Fork 15
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
Workflows #197
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.
LGTM, do note that I didnt' review any of the code/docs/config as they all seem linting things, and I don't have knowledge of them. Couple of small comments on the workflows, but aside from that it should be fine
gh extension install actions/gh-actions-cache | ||
|
||
REPO=${{ github.repository }} | ||
BRANCH="refs/pull/${{ github.event.pull_request.number }}/merge" | ||
|
||
echo "Fetching list of cache key" | ||
cacheKeysForPR=$(gh actions-cache list -R $REPO -B $BRANCH -L 100 | cut -f 1 ) | ||
|
||
## Setting this to not fail the workflow while deleting cache keys. | ||
set +e | ||
echo "Deleting caches..." | ||
for cacheKey in $cacheKeysForPR | ||
do | ||
gh actions-cache delete $cacheKey -R $REPO -B $BRANCH --confirm | ||
done | ||
echo "Done" |
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've got a PR open on core that refactors this and significantly improves testing speed so you might want to copy that over once it lands and is working well. For now this is fine though.
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'll wait a bit (as you suggested) and create a seperate branch/ PR for it.
.github/workflows/test_dev.yml
Outdated
- name: Check style | ||
run: black --check . |
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.
this should not be necessary here as that is tsted by the pre-commit/lining action
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.
Indeed. Was rudimentary code.
.github/workflows/test_stable.yml
Outdated
- name: Check style | ||
run: black --check . |
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.
this should not be necessary here as that is tsted by the pre-commit/lining action
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.
Same.
push: | ||
branches: [ main ] | ||
pull_request: | ||
branches: [ main ] |
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.
Since these are equal to the triggers on test stable, they are going to always run in parallel. if you want test dev to only happen after stable, I think you need to remove this bit
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'll test it. Thanks!
This is still in progess and some stuff is up for discussion: