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.

Sign up for GitHub

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

Jump to bottom

Drop legacy Ruby, Rails, and Webpacker support #163

Merged
merged 4 commits into from
Sep 16, 2024
Merged

Drop legacy Ruby, Rails, and Webpacker support #163

merged 4 commits into from
Sep 16, 2024

Conversation

tagliala
Copy link
Contributor

@tagliala tagliala commented Sep 3, 2024 •
edited
Loading

  • Drop Webpacker support
  • Drop Ruby < 3.1 support
  • Drop Rails < 7.0 support

Close #161, close #162, close #164

@tagliala tagliala force-pushed the feature/161-drop-legacy-ruby-rails branch 3 times, most recently from 8d9bb55 to 13bba98 Compare September 3, 2024 12:16
tagliala
tagliala commented Sep 3, 2024
View reviewed changes
inline_svg.gemspec Outdated Show resolved Hide resolved
@tagliala tagliala force-pushed the feature/161-drop-legacy-ruby-rails branch 6 times, most recently from ba53752 to f5678e4 Compare September 4, 2024 06:41
@tagliala tagliala changed the title Feature/161 drop legacy ruby rails Drop legacy Ruby, Rails, and Webpacker support Sep 4, 2024
@tagliala tagliala marked this pull request as ready for review September 4, 2024 06:42
jamesmartin
jamesmartin reviewed Sep 5, 2024
View reviewed changes
Copy link
Owner

@jamesmartin jamesmartin left a comment

Choose a reason for hiding this comment

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

I'm wondering if it's worth creating a v2 branch in this repo so we can work on the 2.0 breaking changes separately from the current, stable v1.x tree. What do you think? 🤔

@tagliala
Copy link
Contributor Author

tagliala commented Sep 5, 2024

I'm wondering if it's worth creating a v2 branch in this repo so we can work on the 2.0 breaking changes separately from the current, stable v1.x tree. What do you think? 🤔

It will work, but I usually see the opposite: 1-stable branch, and breaking changes on master

@tagliala tagliala force-pushed the feature/161-drop-legacy-ruby-rails branch 2 times, most recently from 08978d5 to c75c0a1 Compare September 5, 2024 07:15
@tagliala tagliala marked this pull request as draft September 5, 2024 09:39
@tagliala
Copy link
Contributor Author

tagliala commented Sep 5, 2024 •
edited
Loading

@jamesmartin

Since the suggested Ruby version for 6.1 is 3.0 and this is starting with 3.1, and 6.1 is going to EOL on 01 Oct 2024, I would also drop 6.1

@jamesmartin
Copy link
Owner

I usually see the opposite: 1-stable branch, and breaking changes on master

Fair enough. Let's go with that approach.

jamesmartin
jamesmartin approved these changes Sep 6, 2024
View reviewed changes
Copy link
Owner

@jamesmartin jamesmartin left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks again for the PR. 👍

@jamesmartin
Copy link
Owner

OK. We're going with v2 changes on master, so this can be merged now. I've created a v1-stable branch for the older version.

@jamesmartin jamesmartin marked this pull request as ready for review September 6, 2024 02:20
@tagliala tagliala marked this pull request as draft September 10, 2024 20:00
@tagliala
Copy link
Contributor Author

I would also drop 6.1

Sorry, I forgot to drop 6.1, will do that and request another review

@tagliala
Drop legacy Ruby and Rails support
73e25fd 
`ERB::Util.html_escape_once` is available in all supported Rails
versions

Also:
- Opt-in for MFA (security)
- Move development gems to Gemfile

Close jamesmartin#161, close jamesmartin#164
@tagliala tagliala force-pushed the feature/161-drop-legacy-ruby-rails branch from c75c0a1 to baa3f6b Compare September 10, 2024 20:07
tagliala
tagliala commented Sep 10, 2024
View reviewed changes
inline_svg.gemspec
Comment on lines +24 to +25
spec.add_dependency "activesupport", ">= 7.0"
spec.add_dependency "nokogiri", ">= 1.16"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the reason why I've removed development_dependency from here and renamed add_runtime_dependency to add_dependency is because they are known offences of rubocop packaging and will be clearer with future commits when this will be merged

@tagliala tagliala marked this pull request as ready for review September 10, 2024 20:09
@tagliala
Improve CI
ada2d15 
- Test against multiple Ruby version
- Experimental tests against ruby-head to spot issues in advance
@tagliala tagliala force-pushed the feature/161-drop-legacy-ruby-rails branch from baa3f6b to 0727f83 Compare September 10, 2024 20:11
@tagliala tagliala force-pushed the feature/161-drop-legacy-ruby-rails branch from 0727f83 to c1445c7 Compare September 10, 2024 20:13
tagliala
tagliala commented Sep 10, 2024
View reviewed changes
README.md
- [Rails 7](https://weblog.rubyonrails.org/2021/12/6/Rails-7-0-rc-1-released/)

Inline SVG no longer officially supports Rails 3 or Rails 4 (although they may still work). In order to reduce the maintenance cost of this project we now follow the [Rails Maintenance Policy](https://guides.rubyonrails.org/maintenance_policy.html).
Inline SVG supports Rails 7.x with Propshaft, Sprockets, or Shakapacker
Copy link
Contributor Author

@tagliala tagliala Sep 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Propshaft was missing I guess

jamesmartin
jamesmartin approved these changes Sep 16, 2024
View reviewed changes
Copy link
Owner

@jamesmartin jamesmartin left a comment

Choose a reason for hiding this comment

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

Thanks, @tagliala. I'm grateful for your help getting to a place where we can drop support for older versions of Rails. ✨

@jamesmartin jamesmartin merged commit cb01bb3 into jamesmartin:main Sep 16, 2024
6 checks passed
@tagliala
Copy link
Contributor Author

Welcome, I will proceed with some other changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jamesmartin jamesmartin jamesmartin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@tagliala @jamesmartin