Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump dependencies and CI Ruby versions #493

Merged
merged 6 commits into from
Nov 21, 2022
Merged

Bump dependencies and CI Ruby versions #493

merged 6 commits into from
Nov 21, 2022

Conversation

ashmaroli
Copy link
Member

@ashmaroli ashmaroli commented Nov 20, 2022

Closes #492
Closes #494

@ashmaroli
Copy link
Member Author

Looks like we have reached a stalemate.. :-(

@parkr
Copy link
Member

parkr commented Nov 20, 2022

Maybe we need to make this about dropping EOL Rubies and make a separate CL about adding Ruby 3 support. What do you think?

@parkr
Copy link
Member

parkr commented Nov 20, 2022

Another thing to consider is limiting old importers to a specific ruby version. We can add runtime checks and documentation to say a given importer is limited to Ruby 2.x. It’s not ideal but this gem and its importers are best-effort.

@ashmaroli ashmaroli changed the title Stop testing with EOL Ruby versions Bump dependencies and CI Ruby versions Nov 21, 2022
@ashmaroli ashmaroli requested a review from parkr November 21, 2022 13:07
Copy link
Member

@parkr parkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple cleanup comments. Thanks for tackling this!

s.add_development_dependency("simplecov", "~> 0.7")
s.add_development_dependency("simplecov-gem-adapter", "~> 1.0")

# migrator dependencies:
s.add_development_dependency("behance", "~> 0.3")
# s.add_development_dependency("behance", "~> 0.3") # uses outdated dependencies
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we move this to the Gemfile and add a Ruby version check or just wait until someone comes around and tries to add tests for the Behance importer?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shall wait for someone to add tests for the Behance importer or at minimum report a bug or some other issue with the Behance importer..
I'm not keen on moving dev_deps to Gemfile (for this project) because the Rubygems dev_deps listing may serve as secondary documentation for users.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Commenting this out removes it from the dev deps anyway, so I'm not sure it's of much use here.

test/helper.rb Outdated
@@ -17,7 +17,7 @@ class Test::Unit::AssertionFailedError < ActiveSupport::TestCase::Assertion
require File.expand_path("../lib/jekyll-import.rb", __dir__)
include JekyllImport

JekyllImport::Importer.subclasses.each(&:require_deps)
# JekyllImport::Importer.subclasses.each(&:require_deps)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove this line.

@ashmaroli
Copy link
Member Author

@jekyllbot: merge +dev

@jekyllbot jekyllbot merged commit 3672f8d into master Nov 21, 2022
@jekyllbot jekyllbot deleted the drop-eol-rubies branch November 21, 2022 17:04
jekyllbot added a commit that referenced this pull request Nov 21, 2022
@jekyll jekyll locked and limited conversation to collaborators Nov 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Ruby 3.x support Remove EOL Rubies from CI
3 participants