The Wayback Machine - https://web.archive.org/web/20201010163441/https://github.com/github/rubocop-github/issues/28
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

Is "avoid using String#+" still valid in the era of frozen_string_literal #28

Open
latentflip opened this issue Jun 5, 2018 · 2 comments
Open

Comments

@latentflip
Copy link

@latentflip latentflip commented Jun 5, 2018

I was just digging through the styleguide, and spotted that at least in github/github where we fail the build if frozen_string_literal: true is not set, the suggested code won't actually run.

Should we update the guidance here? Or be more clear that large is the keyword here, and guide as to how to correctly opt out of frozen_string_literal (I don't actually know how to)


Avoid using String#+ when you need to construct large data chunks. Instead, use String#<<. Concatenation mutates the string instance in-place and is always faster than String#+, which creates a bunch of new string objects.

# good and also fast
html = ""
html << "<h1>Page title</h1>"

paragraphs.each do |paragraph|
  html << "<p>#{paragraph}</p>"
end
@rmosolgo
Copy link
Member

@rmosolgo rmosolgo commented Sep 5, 2018

👋 I guess this project is pretty quiet. I agree this cop needs an update, I'll take a look as part of #29

@kytrinyx
Copy link
Contributor

@kytrinyx kytrinyx commented Apr 24, 2020

guide as to how to correctly opt out of frozen_string_literal

At the top of the file, you can add a comment:

# rubocop:disable Style/FrozenStringLiteralComment

This would be in place of:

# frozen_string_literal: true
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.
Morty Proxy This is a proxified and sanitized view of the page, visit original site.