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

Document support for frozen_string_literal: false #7429

Merged
merged 1 commit into from Oct 21, 2019
Merged

Document support for frozen_string_literal: false #7429

merged 1 commit into from Oct 21, 2019

Conversation

gfyoung
Copy link
Contributor

@gfyoung gfyoung commented Oct 14, 2019

xref #7426

@gfyoung
Copy link
Contributor Author

gfyoung commented Oct 14, 2019

[1 / 2]

This is my first time contributing to the rubocop project, so I'm not 100% sure about commit messages. My PR is not "fixing" the issue (it's a two-parter, and this only addresses one part of it), so I wasn't sure that I needed to add a "fix" prefix to my commit message (FYI: my current commit summary barely makes the cutoff at 49 characters).

Let me know what makes sense here!

[2 / 2]

As I was writing this up, two questions:

  • Does it make sense to include an example showing frozen_string_literal: false ?
  • Should the error message from rubocop be updated to indicate we also support false (my fear is that people might be scared that they have to add true because rubocop said so) ?

Copy link
Collaborator

@Drenmi Drenmi left a comment

Choose a reason for hiding this comment

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

🙏

@bbatsov
Copy link
Collaborator

bbatsov commented Oct 18, 2019

The comment seems a bit confusing to me. Shouldn't it say that RuboCop will ignore files where the the comment exists and is set to false instead?

@gfyoung
Copy link
Contributor Author

gfyoung commented Oct 18, 2019

The comment seems a bit confusing to me. Shouldn't it say that RuboCop will ignore files where the the comment exists and is set to false instead?

@bbatsov : We can write it like that as well. I just want to make it clear that users have an alternative if they have the cop turned on (the error message from the cop does not indicate that)

@gfyoung
Copy link
Contributor Author

gfyoung commented Oct 18, 2019

@bbatsov : I've updated the wording and included an example. Have a look.

@bbatsov bbatsov merged commit 4254691 into rubocop:master Oct 21, 2019
@bbatsov
Copy link
Collaborator

bbatsov commented Oct 21, 2019

Looks good to me. Thanks!

@gfyoung gfyoung deleted the frozen-string-literal-false-docs branch October 21, 2019 07:46
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.

None yet

3 participants