rails-sqlserver / tiny_tds

TinyTDS - Simple and fast FreeTDS bindings for Ruby using DB-Library.
Other
605 stars 191 forks source link

Drop support for SQL server < 2017 #554

Closed andyundso closed 3 months ago

andyundso commented 3 months ago

With this PR, we drop support for SQL Server < 2017.

The first part of the PR extends our existing Continous Integration to run tests against each combination of our supported Ruby versions and SQL Server 2017, 2019 and 2022 on both Linux and Windows.

I therefore would also suggest we remove support for anything below SQL Server 2017. Technically, people could still use tiny_tds against these older versions as long as they compile FreeTDS with TDS v7.3 or v7.4. And SQL Server 2008 is the earliest version that supports v7.3. However, I think it's good if we can test our software against what we support, and if 2017 is the earliest version where I can do this, then I'm happy to drop support for anything older. Only v2014 and v2016 are still covered by extended maintenance of Microsoft, we can expect most people upgraded at this point.

The second part of this PR is code removal. First of all, there was not any new data type or changes to them since SQL Server 2016, so I removed all the different schemas. I also removed two ifdef from the C code.

People should get a compile error when trying to build tiny_tds now with an version older than v1.0 when building tiny_tds.

Two things I'd like to tackle in the future.

andyundso commented 3 months ago

@aharpervc would you mind to re-review. besides implementing your feedback, I also optimised two things in the C.

andyundso commented 3 months ago

@aharpervc I ran out of free credits on CircleCI thanks to this PR. It is somewhat expected since this PR adds many more parallel pipeline runs, but still happened faster than I anticipated.

I see two options:

  1. Revert most changes here in regards to the pipeline, means we continue to only test on MSSQL 2017.
  2. Move to GitHub Actions since you have unlimited runtime when the repository is public. I already built of copy of our pipeline last year in a fork of ours, so most work is already done, just needs to updated again (drop the older Ruby builds, add 3.3, update rake-compiler-dock).

I personally grew very found of GitHub Actions, mainly because of the actions. At work, we are moving all projects over from Semaphore CI. But it was you in collaboration with @bryanwieg who got the CircleCI pipeline up-and-running, so I would leave the choice going forward to you.

bryanwieg commented 3 months ago

I would not take it personally at all if the pipeline was changed to GitHub. Even when I worked on it so long ago, trying to manage the CircleCI credits was a constant battle. Besides being free, GitLab is a widely used integrated and supported solution, whereas I’ve never again used CircleCI outside this project.

aharpervc commented 3 months ago

But it was you in collaboration with @bryanwieg who got the CircleCI pipeline up-and-running, so I would leave the choice going forward to you.

Also fine with me to set up a PR to switch to GH Actions, for whoever has time to work on it. Not sure if you're saying it needs to be a prerequisite for this PR though, I'd personally prefer that be separate work. But that'd also give us all a chance to try and imagine a CI workflow that's less complicated, if such a thing is possible.

andyundso commented 3 months ago

Not sure if you're saying it needs to be a prerequisite for this PR though, I'd personally prefer that be separate work

Absolutely not, I just can't re-run the one failing test (downloading and installing Ruby on Windows timed out) since I do not have any credits left. But I would go ahead and merge this PR before building the GitHub Actions part next.