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

Better parallelize CI jobs #1507

Merged
merged 2 commits into from
Oct 10, 2024
Merged

Conversation

Earlopain
Copy link
Collaborator

Drop appraisal while we're at it. Makes it a bit easier

Copy link
Owner

@bensheldon bensheldon left a comment

Choose a reason for hiding this comment

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

💯 🥳 🥇

This is amazing!!!! THANK YOU!! You cleaned up so many problems and it's so much better now (and TIL eval_gemfile).

I'm a little surprised that there aren't more gem tweaks that are necessary because I felt like I was frequently struggling with Appraisal, so this is great!

@@ -186,13 +174,13 @@ jobs:
uses: actions/upload-artifact@v4
if: failure()
with:
name: screenshots
name: screenshots-${{ matrix.pg }}-${{ matrix.gemfile }}-${{ matrix.ruby }}
Copy link
Owner

Choose a reason for hiding this comment

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

you fixed a longstanding annoyance! 🙌🏻

@@ -86,22 +86,39 @@ jobs:
path: demo/log

test:
name: Test
name: Test - Ruby ${{ matrix.ruby }} - Postgres ${{ matrix.pg }} - ${{ matrix.gemfile }}
Copy link
Owner

Choose a reason for hiding this comment

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

I think Rails is likely to be the culprit in most matrix-related failures:

Suggested change
name: Test - Ruby ${{ matrix.ruby }} - Postgres ${{ matrix.pg }} - ${{ matrix.gemfile }}
name: Test - ${{ matrix.gemfile }} - Ruby ${{ matrix.ruby }} - PG ${{ matrix.pg }}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, that makes sense. I'll fix that up

@bensheldon
Copy link
Owner

btw, that failure with Rails 6.1 is sorta known: #1280

It's fixed by pulling from the Rails 6.1-stable git branch, though I'm a little surprised it shows up now rather than before.

@Earlopain
Copy link
Collaborator Author

btw, that failure with Rails 6.1 is sorta known:

6.1 previously only ran with Ruby 3.0 which explains why it didn't pop up before. I could add excludes to the matrix to preserve that but just pulling 6-1-stable seems fine for CI. WDYT?

* matrix: teamcapybara/capybara#2468
* `nokogiri 🤷 but there's no version constraint anyways
* `net-*`: mikel/mail#1472

It's been quite a while so these seem safe to drop
Ditch appraisals while we're at it
@Earlopain
Copy link
Collaborator Author

For eval_gemfile, it sorta is/isn't actually endorsed for usage by the bundler maintainers rubygems/rubygems#6292 (comment) rubygems/bundler-site#1081. It makes things so very simple though and don't believe it'll disappear (and if it does, let's just handle that then)

(I'm hugely surprised that this PR turned out well with just one try. Usually my interactions with github actions require much more force-pushes 🙃)

@Earlopain Earlopain marked this pull request as ready for review October 10, 2024 13:52
@bensheldon bensheldon merged commit b8faa6c into bensheldon:main Oct 10, 2024
46 of 47 checks passed
@bensheldon
Copy link
Owner

🫶 thank you!

@Earlopain Earlopain deleted the ci-better-matrix branch October 10, 2024 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

2 participants