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

Remove iconv conditional require #170

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

tagliala
Copy link
Contributor

@tagliala tagliala commented Dec 12, 2024

Drop legacy character encoding support for Ruby versions prior to 1.9, as string encoding is now natively supported in modern Ruby versions.

Why and what is being done.

Pre-Merge Checklist

  • CHANGELOG.md updated with short summary

Drop legacy character encoding support for Ruby versions  prior to 1.9,
as string encoding is now natively supported in modern Ruby versions.
Copy link
Contributor

@grosser grosser left a comment

Choose a reason for hiding this comment

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

❤️

@grosser grosser merged commit 3cea380 into premailer:master Dec 12, 2024
6 checks passed
@grosser
Copy link
Contributor

grosser commented Dec 12, 2024

1.20.0

@tagliala tagliala deleted the chore/remove-iconv branch December 12, 2024 16:12
@tagliala tagliala restored the chore/remove-iconv branch December 12, 2024 16:12
@tagliala tagliala deleted the chore/remove-iconv branch December 12, 2024 16:12
@tagliala
Copy link
Contributor Author

tagliala commented Dec 12, 2024

@grosser I'm seeing that there are several string =~ regexp that can be converted to regexp.match?(string) to improve performance and eliminate memory consumption

There is also a rubocop/performance cop with safe autofix for this use case, are you interested in this change?

@grosser
Copy link
Contributor

grosser commented Dec 12, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants