Faveod / arel-extensions

Extending Arel
MIT License
142 stars 24 forks source link

mssql: support rails 7 #107

Closed stackmystack closed 2 months ago

jdelporte commented 2 months ago

Everything here is done more or less for the same reason PR #108 was made, where literal date string (in sqlserver) are transformed into date object, Why is this revert ?

jdelporte commented 2 months ago

We should test on different mssql versions

stackmystack commented 2 months ago

Everything here is done more or less for the same reason PR #108 was made, where literal date string (in sqlserver) are transformed into date object, Why is this revert ?

It's not technically a revert, because otherwise the tests from the PR would not pass.

Instead of formatting manually, we're using the to_formatted_s, which I picked up by reading arel itself. This allows us to align the patching we're doing with the original code, so maintaining those 2 copies becomes far less painful, as divergence can be eye-balled in a couple of seconds.

We should test on different mssql versions

Absolutely agreed. I tried to add 2022 for instance, and that was a failure, so I left it as is. I will have to look (yet again) for how to support more versions.

jdelporte commented 2 months ago

For the first point: Ok then.

For the versions, I was more thinking about older versions. But I guess we will need to run windows server images.

stackmystack commented 2 months ago

For the versions, I was more thinking about older versions. But I guess we will need to run windows server images.

Yeah, MS doesn't provide old images, and it might be problematic that we do them ourselves. We might have to go (back) the travis route 😵‍💫.