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 SqlComments #334

Merged
merged 2 commits into from
May 13, 2024
Merged

Fix SqlComments #334

merged 2 commits into from
May 13, 2024

Conversation

HeyNonster
Copy link
Contributor

@HeyNonster HeyNonster commented May 12, 2024

SqlComments is broken in Rails 7.0 because it now passes an async kwarg1 when it used to only have the two positional arguments.

I've updated the tests to... actually test with Rails. Before it didn't run any Rails code at all.

Releases the patch as v5.5.1.

Footnotes

  1. (https://github.com/rails/rails/blob/5873d6279a2b2ec11d0176da66df17b076733791/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb#L194)

@HeyNonster HeyNonster force-pushed the nony--fix-rails-7-0 branch from 9cca5f7 to 2bd37ef Compare May 12, 2024 16:22
@HeyNonster HeyNonster marked this pull request as ready for review May 12, 2024 16:25
@HeyNonster HeyNonster requested review from a team as code owners May 12, 2024 16:25
`SqlComments` is broken in Rails 7.0 because it now passes an `async`
kwarg[^1] when it used to only have the two positional arguments.

[^1]:(https://github.com/rails/rails/blob/5873d6279a2b2ec11d0176da66df17b076733791/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb#L194)

The reason I'm using a sublass of `Mysql2Adapter` is so that we can
prepend `SqlComments` onto it and use the instance of that custom class
to test that the comments are added without adding comments to the rest
of the test suite.

I've updated the tests to... actually test with Rails. Before it didn't
run any Rails code at all.
@HeyNonster HeyNonster force-pushed the nony--fix-rails-7-0 branch from 2bd37ef to 2d78503 Compare May 13, 2024 00:33
@HeyNonster HeyNonster merged commit 3c30e44 into main May 13, 2024
17 checks passed
@HeyNonster HeyNonster deleted the nony--fix-rails-7-0 branch May 13, 2024 00:38
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.

3 participants