-
Notifications
You must be signed in to change notification settings - Fork 27
Set max-parallel for all GitHub action jobs #1602
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
Merged
stevedlawrence
merged 1 commit into
apache:main
from
stevedlawrence:daffodil-3069-github-action-max-parallel
Mar 3, 2026
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Note that the matrix for this job results in 44 jobs. With a max-parallel of 15, this is expected to take approximately 3 times longer. Maybe that's fine if each job is only a couple of minutes. But it might be worth taking a look at this matrix to see if all of these jobs are really necessary, or if some could be removed and still provide confidence that a PR does not break anything.
Uh oh!
There was an error while loading. Please reload this page.
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.
@stevedlawrence Do you think possibly removing ubuntu or macos from some would be okay? Basically doing the assumption that if it works on Ubuntu or MacOS it should work on the other or what would you suggest?
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.
Sure, daffodil only has one job for macOS so cutting back some of the OS matrices seems reasonable to me. Especially since typescript and Java are mostly OS agnostic.
The main issue we frequently run into with OS differences is related to paths (e.g. slashes, spaces, newlines). And those are usually Java/typescript version agnostic. So also long as you have at least one job for each OS, one job for each Java version, typescript version, etc. you probably can get away with having a fairly sparse matrix.
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.
Do you think that should be done in a different PR or in this one?
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 don't have a preference whether it's part of this PR or separate. I'll leave it up the the daffodil-vscode devs.
If you'd like to include the changes in this PR, feel free to push what you decide to exclude to this PR branch.