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

Fix syntax for some if-conditions in ci.yaml #11109

Merged
merged 1 commit into from
Apr 23, 2024

Conversation

AlexWaygood
Copy link
Member

Summary

These CI jobs always fail on my fork whenever I sync my fork with the upstream repo. I have the power to create branches on the upstream ruff repo these days but I still occasionally sync and push branches to my fork out of habit, so this is kinda annoying 😛

Comment on lines -276 to -278
if: github.event_name == 'pull_request' && ${{
needs.determine_changes.outputs.code == 'true'
}}
Copy link
Member Author

Choose a reason for hiding this comment

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

The syntax is incorrect here; this currently seems to always evaluate to true when I sync the main branch of my fork with the upstream main

@@ -554,7 +552,7 @@ jobs:
benchmarks:
runs-on: ubuntu-latest
needs: determine_changes
if: ${{ needs.determine_changes.outputs.code == 'true' || github.ref == 'refs/heads/main' }}
if: ${{ github.repository == 'astral-sh/ruff' && (needs.determine_changes.outputs.code == 'true' || github.ref == 'refs/heads/main') }}
Copy link
Member Author

Choose a reason for hiding this comment

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

This should be skipped for forks because our codspeed token is only valid for astral-sh/ruff.

This is quite a long line. I think there are ways of doing multiline if-conditions in GitHub Actions, but they seem to be pretty poorly documented, and I've got it wrong frequently in the past. I'd prefer to just do a very long single-line if-condition, as I currently have.

Copy link
Contributor

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@AlexWaygood AlexWaygood added the ci Related to internal CI tooling label Apr 23, 2024
@AlexWaygood AlexWaygood merged commit 455d22c into astral-sh:main Apr 23, 2024
18 checks passed
@AlexWaygood AlexWaygood deleted the forks branch April 23, 2024 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Related to internal CI tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants