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

Support Rails 8.0 #101

Merged
merged 2 commits into from
Nov 15, 2024
Merged

Support Rails 8.0 #101

merged 2 commits into from
Nov 15, 2024

Conversation

zhuravel
Copy link
Contributor

@zhuravel zhuravel commented Nov 9, 2024

Issue #, if available:

Description of changes:

  • Add support for Rails 8.0
  • Add Ruby 3.3, Rails 8.0, PostgreSQL 17, MySQL 9.1 to tests
  • Remove MySQL 5.5 from tests as its support ended in 2018 and it’s not supported by Rails 8.0
  • Upgrade versions of GitHub actions to fix deprecation warnings

Important

The only test failing is this one:

Please make sure test coverage is 100%. Currently: 100% (line), 97.14% (branch).

Not sure why branch coverage decreased, any suggestions are welcome.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@dmytro-savochkin
Copy link
Collaborator

Hey @zhuravel, thanks for the great PR! Appreciate it!

I have two comments:

  1. Could we actually keep Gemfile kinda empty as it was before? At least to me, it feels more obvious this way when every dependency for a gem is defined inside .gemspec file. And this change is not strictly necessary for anything in this PR.
  2. I'd really like to get down to what caused the branch test coverage decrease but I don't have time to research it right now, so I will try to look at it a bit later the next week.

@dmytro-savochkin
Copy link
Collaborator

Okay, I've done some debugging. Apparently the problem is with collating Simplecov does to calculate branch test coverage across different ruby and rails versions. We have to do it because of this line.
However I don't understand what exactly causes it. And as it turns out if I push the exact same version of code that was green a year ago, now it's red. So it has nothing to do with the changes in this PR. I think I am gonna just merge it and decrease the coverage threshold.

@dmytro-savochkin dmytro-savochkin merged commit 13f93c2 into jpignata:master Nov 15, 2024
32 of 33 checks passed
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.

2 participants