Skip to content

Adapt code to frozen string literal, reduce string copies#456

Open
czj wants to merge 1 commit intokjvarga:masterfrom
levups:adapt-to-frozen-string-literals
Open

Adapt code to frozen string literal, reduce string copies#456
czj wants to merge 1 commit intokjvarga:masterfrom
levups:adapt-to-frozen-string-literals

Conversation

@czj
Copy link
Copy Markdown

@czj czj commented Jul 28, 2025

Avoids last frozen string literal warnings by implying the passed string will always be frozen in upcoming Ruby versions.

Also reduces object allocations, including string copies, arrays, and maps, backported partly from #445 which it replaces.

@czj czj mentioned this pull request Jul 28, 2025
@mariozig
Copy link
Copy Markdown

mariozig commented Aug 2, 2025

hi @kjvarga are you open to taking this?

@n-rodriguez
Copy link
Copy Markdown
Collaborator

@czj hello! I'm a bit nervous to merge this PR since CI didn't run. Do you know how to trigger CI? Thank you!

@czj
Copy link
Copy Markdown
Author

czj commented Sep 2, 2025

@czj hello! I'm a bit nervous to merge this PR since CI didn't run. Do you know how to trigger CI? Thank you!

Hi! I don't know why the CI didn't trigger automatically for this particular repo. Is it a configuration issue with a forked repo, or a problem on my side?

@n-rodriguez n-rodriguez closed this Sep 9, 2025
@n-rodriguez n-rodriguez reopened this Sep 9, 2025
@n-rodriguez
Copy link
Copy Markdown
Collaborator

@czj I finally managed to run the CI : it's not all green 😢

@czj
Copy link
Copy Markdown
Author

czj commented Sep 9, 2025 via email

@n-rodriguez n-rodriguez self-assigned this Sep 15, 2025
@joevandyk
Copy link
Copy Markdown

joevandyk commented Sep 16, 2025

might be worth not supporting ruby versions < 3 ?

@n-rodriguez
Copy link
Copy Markdown
Collaborator

might be worth not supporting ruby versions < 3 ?

@kjvarga wdyt?

@kjvarga
Copy link
Copy Markdown
Owner

kjvarga commented Sep 16, 2025

@n-rodriguez I'm okay with it for a new major release. It'll simplify some things, which is a plus.

@joevandyk
Copy link
Copy Markdown

I vote for sitemap_generator 7.0!

The latest release is 6.3 from 2022 right?

@czj
Copy link
Copy Markdown
Author

czj commented Sep 17, 2025

might be worth not supporting ruby versions < 3 ?

Indeed, only 3.2, 3.3, and 3.4 are supported as of 2025.
Extending support to 3.0 seems reasonable.
That would make work on this branch easier :)

@czj
Copy link
Copy Markdown
Author

czj commented Sep 19, 2025

Let me know if you want to drop support for Ruby < 3.0 and I’ll remove it from CI. If you’d prefer, I can also adapt the code for Ruby 2.6+. I’m happy to do either :)

@Lukom
Copy link
Copy Markdown

Lukom commented Nov 15, 2025

I think support for < 3.0 should be removed anyway, as older rubies do not receive security updates:

Version Support Status End Date (Security)
3.4 Normal Maintenance March 31, 2027
3.3 Security Maintenance March 31, 2026
3.2 Security Maintenance March 31, 2026
3.1 Ended March 31, 2025
3.0 Ended April 23, 2024
2.7 Ended March 31, 2023

PR looks good to me.

Although, to fix CI issues with older rubies I think it's enough to leave plus here:

@xml_wrapper_start = +<<-HTML

Also it'd be better to use squiggly heredoc <<~HTML (which was introduced in ruby 2.3 so it won't hurt)

@n-rodriguez
Copy link
Copy Markdown
Collaborator

n-rodriguez commented Feb 5, 2026

hi there! sorry for the delayed response... IMHO we should release a 6.4.0 first, so users can benefit from :

Then we can release a 7.0.0 which drops everything EOL and this PR (and others) merged.

@kjvarga @czj @Lukom wdyt?

@czj
Copy link
Copy Markdown
Author

czj commented Feb 5, 2026

@kjvarga @czj @Lukom wdyt?

LTGM !

@n-rodriguez
Copy link
Copy Markdown
Collaborator

@kjvarga everything is ready for a 6.4.0 release. Thank you!

@kjvarga
Copy link
Copy Markdown
Owner

kjvarga commented Feb 7, 2026

Sounds good! Will email you

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.

6 participants