Skip to content

Conversation

@jonahgeorge
Copy link
Contributor

@jonahgeorge jonahgeorge commented Jul 2, 2024

Hey all,

I've been working on adding SQL Server support to ankane/groupdate and discovered that this patch of ActiveRecord::Relation#calculate is interfering with the library's own patching of the same method.

It appears that all tests pass without this patch so posting this PR to potential remove it as I don't fully follow why it was introduced to start.

@aidanharan aidanharan merged commit 1beb276 into rails-sqlserver:main Jul 2, 2024
@aidanharan
Copy link
Contributor

aidanharan commented Jul 2, 2024

The ActiveRecord::Relation#calculate patch was introduced by #1090 to fix an issue with failing activerecord/test/cases/null_relation_test.rb tests. Not sure exactly what tests now and since the tests pass now without the patch then all good 👍

@jonahgeorge The main branch is used for Rails 7.2 development. Do you need the patch removed from the 7.1 adapter?

@jonahgeorge
Copy link
Contributor Author

The ActiveRecord::Relation#calculate patch was introduced by #1090 to fix an issue with failing activerecord/test/cases/null_relation_test.rb tests. Not sure exactly what tests now and since the tests pass now without the patch then all good 👍

@jonahgeorge The main branch is used for Rails 7.2 development. Do you need the patch removed from the 7.1 adapter?

Would you accept backports for both 7.1 and 7.0? Unfortunately the app that I really need this in is still on 7.0-- No worries if not

@jonahgeorge jonahgeorge deleted the remove-calculate-patch branch July 3, 2024 00:18
@aidanharan
Copy link
Contributor

Backports welcome 👍

@aidanharan
Copy link
Contributor

@jonahgeorge Released v7.0.7 and v7.1.4 with the back ports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants