Data-Liberation-Front / csvlint.rb

The gem behind http://csvlint.io
MIT License
283 stars 86 forks source link

Test over multiple ActiveSupport versions (with Appraisal) #289

Closed D-system closed 6 months ago

D-system commented 6 months ago

Test ActiveSupport from 5.2 to 7.0 over ruby version 3.0 to 3.3. It ensure that all versions are care of.

The matrix of tests is: ActiveSupport 5.2: Ruby 2.5, 2.6, 2.7, 3.0, 3.1, 3.2 and 3.3 ActiveSupport 6.0: Ruby 2.5, 2.6, 2.7, 3.0, 3.1, 3.2 and 3.3 ActiveSupport 6.1: Ruby 2.5, 2.6, 2.7, 3.0, 3.1, 3.2 and 3.3 ActiveSupport 7.0: Ruby 2.7, 3.0, 3.1, 3.2 and 3.3 (Ruby 2.7+ required)

With the current limitation of ActiveSupport 7.0 max (from #279), when ActiveSupport 7.1 is supported we will have tests for 7.0 and below.

D-system commented 6 months ago

One of the dependency does not support Ruby 3+

SCR-20240204-sbda

There is a clear trade off:

D-system commented 6 months ago

Other libraries are incompatible

SCR-20240204-slzs
D-system commented 6 months ago

At the end, I went to add more tests (not changing the current tests) and limit to Ruby 3.0+ due to too many incompatibilities with dependencies.

With this addition, we will have insurance that old versions are working once ActiveSupport 7.1 is added.

D-system commented 6 months ago

@Floppy I know the project was not using Appraisal and that add one more thing to think and know. I would be happy to help on spreading the knowledge even it is the first for myself too to use Appraisal.

If there is a need to a maintainer to maintain the code (no feature implementation), I would be happy to help. I would like to update the code to comply to StandardRb (remove the exceptions) and rely on the CSV gem version instead of the Ruby version (with the current PR that test against multiple Ruby version, it is safer to do that change).

Floppy commented 6 months ago

Sorry for the delay! This is super cool work, thank you. I've not used Appraisal before, but testing across multiple gem dependency versions is fantastic, so I'm happy for it to be added. Only question is, do we need to include the lockfiles in the repo, or should they (can they) be .gitignored?

D-system commented 6 months ago

Good question. Let me spy on other gems 💎 to see what’s the consensus. It was one of my concerns when making the commit. I’ll probably add it to the ignore file as it is the behavior of the current test and gemspec.

Floppy commented 6 months ago

I think unless it's important for Appraisal to work properly, we should probably leave them out. 👍🏻

D-system commented 6 months ago

I looked at the reverse dependency https://rubygems.org/gems/appraisal/reverse_dependencies.

Commited:

Not commited:

It seems that most people are committing their gemfiles. I was leaning more to your side (not committing the gemfiles) yesterday but now after my research, I wonder. It is a political decision, there is no bad decision. With this research, I would like you @Floppy to confirm you want to let them out.

Floppy commented 6 months ago

Ah, sorry, I was unclear. I think the gemfiles themselves should be there, but the lockfiles can be left out - looks like that's what others are doing. Can you remove those and add to .gitignore?

D-system commented 6 months ago

@Floppy Thank you for the explanation, I totally misread your message. Sorry about that.

I removed the lockfiles.

I excluded Ruby 2.5 and 2.6 from ActiveSupport 7.0 due to incompatibility issue from the gem:

SCR-20240221-plqg