Skip to content

Support Ruby 3#216

Merged
sarahsehr merged 11 commits intomasterfrom
bundleRuby3Upgrade
Jul 10, 2023
Merged

Support Ruby 3#216
sarahsehr merged 11 commits intomasterfrom
bundleRuby3Upgrade

Conversation

@sarahsehr
Copy link
Copy Markdown
Contributor

@sarahsehr sarahsehr commented Jul 6, 2023

  • Add support for Ruby 3.0, 3.1, and 3.2
  • Appraise Ruby 2.7.5 and 2.7.7
  • Drop support for Ruby < 2.7
  • Drop support for Rails < 6.1

@sarahsehr sarahsehr changed the title Bundle ruby3 upgrade Support Ruby 3 Jul 6, 2023
@sarahsehr sarahsehr force-pushed the bundleRuby3Upgrade branch from bbc1200 to 4914051 Compare July 6, 2023 22:39
@sarahsehr sarahsehr force-pushed the bundleRuby3Upgrade branch from 4914051 to 1e550dd Compare July 6, 2023 23:03
@sarahsehr sarahsehr mentioned this pull request Jul 6, 2023
@sarahsehr sarahsehr force-pushed the bundleRuby3Upgrade branch from 1e550dd to 39d1be5 Compare July 6, 2023 23:12
Drop ruby 2.5.8 and 2.6.6

Co-authored-by: Logan Sears <logan.sears@appfolio.com>
@sarahsehr sarahsehr force-pushed the bundleRuby3Upgrade branch 3 times, most recently from 1afad9d to b5d5f67 Compare July 7, 2023 21:42
sarahsehr and others added 2 commits July 7, 2023 14:57
Also remove some old Rails code

Co-authored-by: Logan Sears <logan.sears@appfolio.com>
@sarahsehr sarahsehr force-pushed the bundleRuby3Upgrade branch from b5d5f67 to a779df4 Compare July 7, 2023 21:58
Diasporism and others added 7 commits July 7, 2023 15:16
There no usages of this class anymore according to Sourcegraph
This version of Rails is EOL.
Rails 6.0 is EOL.

Additionally, Ruby 3 and Psych 4 and YAML aliases are incompatible.
Upgrading Rails to 6.1 and webpacker 5.4 incorporates the fix.
@sarahsehr sarahsehr force-pushed the bundleRuby3Upgrade branch from a779df4 to f1dac4c Compare July 7, 2023 22:16
@sarahsehr
Copy link
Copy Markdown
Contributor Author

@sarahsehr sarahsehr marked this pull request as ready for review July 10, 2023 16:09
@sarahsehr sarahsehr requested a review from dtoms July 10, 2023 16:10
Copy link
Copy Markdown
Contributor

@tristil tristil left a comment

Choose a reason for hiding this comment

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

🍰

@dtoms
Copy link
Copy Markdown
Contributor

dtoms commented Jul 10, 2023

What are the coverage metrics from Simplcov?

@dtoms
Copy link
Copy Markdown
Contributor

dtoms commented Jul 10, 2023

I believe one of the main concerns from @Diasporism was that the Ruby3 parameters changes might mean tests pass incorrectly. I don't see anything specifically addressing that here (not complaining). Do we still think that might be a valid issue?

@sarahsehr
Copy link
Copy Markdown
Contributor Author

What are the coverage metrics from Simplcov?

Are you asking for the numbers or what it does?

@sarahsehr
Copy link
Copy Markdown
Contributor Author

I believe one of the main concerns from @Diasporism was that the Ruby3 parameters changes might mean tests pass incorrectly. I don't see anything specifically addressing that here (not complaining). Do we still think that might be a valid issue?

I'm waiting to hear back from @Diasporism of an example test that passed when it should have failed. Based on my understanding of the gem (I've been a consumer for a long time, even if not an expert on the gem), that shouldn't happen.

@dtoms
Copy link
Copy Markdown
Contributor

dtoms commented Jul 10, 2023

@sarahsehr the coverage numbers.

@sarahsehr
Copy link
Copy Markdown
Contributor Author

@sarahsehr the coverage numbers.

The unit test coverage was ~41%. I looked through the actual files and all the "covered" lines were just the class and method names, not the bodies. Elvis Restly had removed the integration tests, but I added them back because of that low coverage.

@sarahsehr
Copy link
Copy Markdown
Contributor Author

All PropertyApp selenium tests are passing against a reference to this branch. Nothing to directly QA.

@sarahsehr sarahsehr merged commit 1946373 into master Jul 10, 2023
@sarahsehr sarahsehr deleted the bundleRuby3Upgrade branch July 10, 2023 22:50
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.

4 participants