search

perfectline / validates_url

URL Validation for Rails
MIT License
583 stars 113 forks source link

Migrate CI to GitHub Actions #129

Closed petergoldstein closed 2 years ago

petergoldstein commented 2 years ago

Migrate CI to GitHub Actions as Travis CI.org is no longer active.

In addition to adding the GH config, it was necessary to remove the Jeweler dependency to get things running on recent Rubies. In addition, I did some cleanup of the gemspec.

timrogers commented 2 years ago

This seems to have been done in https://github.com/perfectline/validates_url/commit/80a91294ad6388634246d2a755fa78fc84071815.

petergoldstein commented 2 years ago

@timrogers It was, kind of. Because the CI action now in master does not call rm Gemfile.lock, the Ruby 2.5 build fails in CI. I'd suggest either removing Ruby 2.5 or addressing as in this PR.

petergoldstein commented 2 years ago

@timrogers Oh, and the change in master leaves out modern Rubies - 3.0, 3.1. Again this should be addressed.

timrogers commented 2 years ago

@petergoldstein 😓 Good spot! It might be worth creating a PR to fix that if you have time - I'm sure the maintainer would appreciate it.

petergoldstein commented 2 years ago

@timrogers Frankly, this is that PR. It fixes all the issues that the implementation in master didn't fix. If the maintainer wishes to reopen, this PR can simply be merged.

benjamin-ltr commented 2 years ago

I don't think removing the Gemfile.lock is a good practice. I totally agree with you, my PR is not perfect and I added a comment describing this issue but I'm pretty sure there is a better way to fix that.

petergoldstein commented 2 years ago

@benjamin-ltr The PR you submitted (and was merged) is conceptually flawed. There is no common Gemfile.lock that works for Ruby 2.3 - Ruby head. So there are exactly three solutions if you want to handle that range of Rubies:

  1. Remove the Gemfile.lock from the repo entirely
  2. Use a single Gemfile.lock in the repo, and have the CI process remove it so that each Ruby version can generate its own.
  3. Create a dedicated Gemfile for each Ruby and set the BUNDLE_GEMFILE environment variable to the appropriate Gemfile on each run.

Those are the options. Any of them is "fine", although I tend to favor (1) on my gem repos.

petergoldstein commented 2 years ago

And here's a link to everything running green - https://github.com/petergoldstein/validates_url/actions/runs/2320480558

benjamin-ltr commented 2 years ago

I think there is another option. Nokogiri is not really used by this gem, it's a jeweler dependency. We can remove the jeweler dependency and have another workflow for automatic releases that installs this dependency and automatically release this gem on a tag push.

petergoldstein commented 2 years ago

@benjamin-ltr You are assuming that the only problem is Nokogiri. That's incorrect in this case. Rails is in the Gemfile.lock and there is no version of Rails that runs with all Rubies 2.3-3.1

It's also incorrect in general. This is not a validates_url problem. Nor is it a jeweler problem (I don't really know why jeweler is needed in 2022 at all, as noted by my removal). This is an extremely common problem when putting together CI pipelines that span large ranges of Rubies. Either you cut down the supported Rubies or you engage in one of the three strategies mentioned above.

I'll grant you that attempting to trim your dependencies to only those gems that have at least one single version that supports all Rubies from 2.3 to 3.1 is an additional strategy . But it's almost certainly a failing strategy.

Anyway, I'm not really that invested here. Do what you will. I'll leave this here for a bit and, assuming it hasn't been merged, close it on my next forked repo cleanup.