-
Notifications
You must be signed in to change notification settings - Fork 129
Don't run the PyPi job all the time #1363
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
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1363 +/- ##
=======================================
Coverage 82.05% 82.05%
=======================================
Files 203 203
Lines 48863 48863
Branches 8695 8695
=======================================
Hits 40093 40093
Misses 6619 6619
Partials 2151 2151 🚀 New features to boost your workflow:
|
# Run if it's a release, auto-release branch, or if relevant files changed on main | ||
if: | | ||
github.event_name == 'release' || | ||
github.ref == 'refs/heads/auto-release' || |
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 believe we should delete all references to auto-release
AFAIK it predates PyTensor
outputs: | ||
should_run: ${{ steps.filter.outputs.any_changed }} | ||
steps: | ||
- uses: actions/checkout@v4 |
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.
- uses: actions/checkout@v4 | |
- uses: actions/checkout@v4 | |
with: | |
persist-credentials: false |
@@ -17,9 +17,34 @@ concurrency: | |||
cancel-in-progress: true | |||
|
|||
jobs: |
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.
Make zizmor happy about default permissions
jobs: | |
permissions: {} | |
jobs: |
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.
It looks to me like there's potential to group the whole condition of the if
job runs into an output of the check_changes
job. I'm not sure how to do that though. Maybe Claude knows. I've added a suggestion on to kind of hint at what I mean.
should_run: ${{ steps.filter.outputs.any_changed }} | ||
steps: | ||
- uses: actions/checkout@v4 | ||
- uses: dorny/paths-filter@v2 |
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.
- uses: dorny/paths-filter@v2 | |
- uses: dorny/paths-filter@v3 |
(github.ref == 'refs/heads/main' && needs.check_changes.outputs.should_run == 'true') || | ||
(github.event_name == 'pull_request' && needs.check_changes.outputs.should_run == 'true') |
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.
(github.ref == 'refs/heads/main' && needs.check_changes.outputs.should_run == 'true') || | |
(github.event_name == 'pull_request' && needs.check_changes.outputs.should_run == 'true') | |
( | |
(github.ref == 'refs/heads/main' || github.event_name == 'pull_request') | |
&& needs.check_changes.outputs.should_run == 'true' | |
) |
(github.ref == 'refs/heads/main' && needs.check_changes.outputs.should_run == 'true') || | ||
(github.event_name == 'pull_request' && needs.check_changes.outputs.should_run == 'true') |
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.
(github.ref == 'refs/heads/main' && needs.check_changes.outputs.should_run == 'true') || | |
(github.event_name == 'pull_request' && needs.check_changes.outputs.should_run == 'true') | |
( | |
(github.ref == 'refs/heads/main' || github.event_name == 'pull_request') | |
&& needs.check_changes.outputs.should_run == 'true' | |
) |
(github.ref == 'refs/heads/main' && needs.check_changes.outputs.should_run == 'true') || | ||
(github.event_name == 'pull_request' && needs.check_changes.outputs.should_run == 'true') |
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.
(github.ref == 'refs/heads/main' && needs.check_changes.outputs.should_run == 'true') || | |
(github.event_name == 'pull_request' && needs.check_changes.outputs.should_run == 'true') | |
( | |
(github.ref == 'refs/heads/main' || github.event_name == 'pull_request') | |
&& needs.check_changes.outputs.should_run == 'true' | |
) |
(github.ref == 'refs/heads/main' && needs.check_changes.outputs.should_run == 'true') || | ||
(github.event_name == 'pull_request' && needs.check_changes.outputs.should_run == 'true') |
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.
(github.ref == 'refs/heads/main' && needs.check_changes.outputs.should_run == 'true') || | |
(github.event_name == 'pull_request' && needs.check_changes.outputs.should_run == 'true') | |
( | |
(github.ref == 'refs/heads/main' || github.event_name == 'pull_request') | |
&& needs.check_changes.outputs.should_run == 'true' | |
) |
(github.ref == 'refs/heads/main' && needs.check_changes.outputs.should_run == 'true') || | ||
(github.event_name == 'pull_request' && needs.check_changes.outputs.should_run == 'true') |
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.
(github.ref == 'refs/heads/main' && needs.check_changes.outputs.should_run == 'true') || | |
(github.event_name == 'pull_request' && needs.check_changes.outputs.should_run == 'true') | |
( | |
(github.ref == 'refs/heads/main' || github.event_name == 'pull_request') | |
&& needs.check_changes.outputs.should_run == 'true' | |
) |
check_changes: | ||
runs-on: ubuntu-latest | ||
outputs: | ||
should_run: ${{ steps.filter.outputs.any_changed }} |
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.
should_run: ${{ steps.filter.outputs.any_changed }} | |
should_run: | | |
echo "${{ github.event_name == 'release' || | |
github.ref == 'refs/heads/auto-release' || | |
( | |
github.ref == 'refs/heads/main' || github.event_name == 'pull_request') && | |
steps.filter.outputs.any_changed == 'true' | |
) }}" |
This was mostly Claude-generated changes, so appreciate careful review.
Trying to save tons of compute time we wast building the cython-scan extension on the ubuntu wheel for every PR and push to main. Closes #219
📚 Documentation preview 📚: https://pytensor--1363.org.readthedocs.build/en/1363/