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

Static length #2276

Merged
merged 2 commits into from
Oct 6, 2015
Merged

Static length #2276

merged 2 commits into from
Oct 6, 2015

Conversation

rrosenblum
Copy link
Contributor

I came across a number of places in the codebase that call size on a string literal. I recognize that sometimes this seems better than using a magic variable, so I am allowing it in constants.

The failing tests will be fixed by #2274.

@rrosenblum
Copy link
Contributor Author

I am fine with making it a Performance cop. I initially wrote it as a performance cop, but I wasn't sure if it had enough of a performance impact to be considered a performance cop or not.

@rrosenblum rrosenblum force-pushed the static_length branch 10 times, most recently from 902f11f to 942b6c9 Compare September 28, 2015 19:53
@bbatsov
Copy link
Collaborator

bbatsov commented Sep 29, 2015

I like the cop, but I don't like the name, as static is often used in programming with other meaning. Perhaps LiteralSize or FixedSize? Other ideas would be welcome.

@rrosenblum
Copy link
Contributor Author

I like your suggestion about the name change. I am leaning towards FixedSize unless I think of something else.

@rrosenblum rrosenblum force-pushed the static_length branch 4 times, most recently from 57ef6c0 to 891a166 Compare October 5, 2015 01:32
bbatsov added a commit that referenced this pull request Oct 6, 2015
@bbatsov bbatsov merged commit 1c82cba into rubocop:master Oct 6, 2015
@rrosenblum rrosenblum deleted the static_length branch October 22, 2015 20:51
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