Skip to content

Enforce frozen string in Rubocop#29540

Merged
matthewd merged 1 commit intorails:masterfrom
kirs:rubocop-frozen-string
Jul 1, 2017
Merged

Enforce frozen string in Rubocop#29540
matthewd merged 1 commit intorails:masterfrom
kirs:rubocop-frozen-string

Conversation

@kirs
Copy link
Copy Markdown
Member

@kirs kirs commented Jun 23, 2017

As discussed in #29506 (comment), we should be able to enable Style/FrozenStringLiteralComment rule in Rubocop and add the magic comment to all files.

@matthewd @rafaelfranca @pat

@rails-bot
Copy link
Copy Markdown

r? @matthewd

(@rails-bot has picked a reviewer for you, use r? to override)

@pat
Copy link
Copy Markdown
Contributor

pat commented Jun 23, 2017

Thanks for sorting this out @kirs! 👏

@kirs
Copy link
Copy Markdown
Member Author

kirs commented Jun 30, 2017

ping @rafaelfranca - I rebased it against master.

@esparta
Copy link
Copy Markdown
Contributor

esparta commented Jul 1, 2017

Although I really like to see this as reality I'm pretty sure this should be done on waves, by packages or sections, not as a single PR. There's enough time to plan and organize what comes first or later.
I'm not part of the maintainers group and my opinion should be taken as just as the point of view more.

@matthewd matthewd merged commit cfade1e into rails:master Jul 1, 2017
matthewd added a commit that referenced this pull request Jul 1, 2017
matthewd added a commit that referenced this pull request Jul 1, 2017
@matthewd
Copy link
Copy Markdown
Member

matthewd commented Jul 1, 2017

Reverted because I got ahead of myself; #29506 wasn't enough to make this green.

@kirs
Copy link
Copy Markdown
Member Author

kirs commented Jul 1, 2017

I'll work on fixing those failures.

kamipo added a commit to kamipo/rails that referenced this pull request Jul 1, 2017
Since rails#29540, `# frozen_string_literal: true` included original
migration files.
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.

5 participants