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

I love the smell of Rubocop fixes in the morning. #1118

Merged
merged 9 commits into from
May 18, 2015

Conversation

jaredmoody
Copy link
Contributor

Low hanging fruit - style autocorrect fixes for #945

width ||= @style[:diameter] || @style[:width] || (@style[:radius] || 0) * 2
left ||= @style[:left]
top ||= @style[:top]
width ||= @style[:diameter] || @style[:width] || (@style[:radius] || 0) * 2
Copy link
Member

Choose a reason for hiding this comment

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

hm, I do disagree with this - might be my personal style (and that my IDE makes those for me) but I like lining op = operators just as I like lining up => in hashes, tremendously improves readability imo :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing this out, I missed this and I agree with you. Looks like there's a configuration option called MultiSpaceAllowedForOperators that will allow lining these up. I'll drop this commit from the PR and make a new one for this cop.

Copy link
Member

Choose a reason for hiding this comment

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

👍 sounds good

@PragTob
Copy link
Member

PragTob commented May 6, 2015

Most of this looks good 👍

Personally I like aligning operators such as = for multiple assignmetns or something like that, much more readable.

The other things are about spaces around braces, where I'm not entirely sure.

Tobi

@jaredmoody
Copy link
Contributor Author

Added some notes above, I'll move the questionable commits into their own PRs

@jaredmoody jaredmoody force-pushed the rubocop branch 3 times, most recently from 19a31fd to 45290ed Compare May 11, 2015 07:40
@jaredmoody
Copy link
Contributor Author

Rebased against master and bumped some method length/complexity limits from new code in master :P

@jasonrclark jasonrclark modified the milestone: 4.0.0.pre5 May 12, 2015
@jaredmoody
Copy link
Contributor Author

Rebased against master after pre4 merge.

@@ -79,16 +79,16 @@ def run(str)
when "\n"
begin
out, obj = IRBalike.run(@cmd + ';')
@str += ["#@cmd\n",
@str += ["#{@cmd}\n",
Copy link
Member

Choose a reason for hiding this comment

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

Didn't even realize this was allowed with strings! Glad to see it going away 😁

@jasonrclark jasonrclark merged commit a455fd7 into shoes:master May 18, 2015
@jasonrclark
Copy link
Member

Reading through this, I'm happy with the hash changes as they stand so I'll take it.

@jaredmoody jaredmoody deleted the rubocop branch May 18, 2015 06:47
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.

3 participants