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 when GHAs run #1110

Merged
merged 3 commits into from
Jun 23, 2022
Merged

Conversation

jayofdoom
Copy link
Contributor

Aiming to make it run on pushes of all branches (except master, which
do not need to run tests after merge -- this will change when releasing
is moved to GHA).

Should also fix issue where PRs from forks don't have the job runners
show up in the list at the bottom of the pull request.

Related: Issue #1037

Aiming to make it run on pushes of all branches (except master, which
do not need to run tests after merge -- this will change when releasing
is moved to GHA).

Should also fix issue where PRs from forks don't have the job runners
show up in the list at the bottom of the pull request.

Related: Issue armadaproject#1037
Copy link
Contributor

@kannon92 kannon92 left a comment

Choose a reason for hiding this comment

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

Can we keep a single list where we say what branches we want to kick jobs of for?

Ideally, a list we can just reference rather than copying it into two different places in each workflow.

@jayofdoom
Copy link
Contributor Author

The only opportunity to DRY up, AFAICT, would be using yaml anchors to make us only list the paths once per workflow. That adds a bit of complexity (someone reading this file has to grok yaml anchors) for very minimal benefit. I would go the route of DRYing it up if I could do it across github workflows.

If you feel strongly about it; I can use the yaml anchors but for small config snippets like this, I find the readability of being explicit worth the tradeoff.

@severinson severinson merged commit cc09345 into armadaproject:master Jun 23, 2022
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.

None yet

5 participants