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

Use Rubocop #32

Closed
wants to merge 40 commits into from
Closed

Use Rubocop #32

wants to merge 40 commits into from

Conversation

mvidner
Copy link
Member

@mvidner mvidner commented Nov 26, 2014

No description provided.

rubocop --auto-gen-config
removed dead code this way :-)
Conflicts:
	.rubocop.yml
# The point is for the user to remove these configuration records
# one by one as the offenses are removed from the code base.
# Note that changes in the inspected code, or installation of new
# versions of RuboCop, may require this file to be generated again.
Copy link
Member

Choose a reason for hiding this comment

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

I think in final rubocop this comment should be removed as it just confuse readers.

Style/HashSyntax: do enforce it, for new code
Style/MultilineOperationIndentation: indented is right
Style/VariableName: Style/MethodName: do not spell out snake_case
  which is the default
The impact of the bug was small because other code queried DNS anyway and
filtered out the nonexistent host name.
"For <b>Mount Point</b>, enter the path in the local file system where the directory should be mounted. With\n" +
"<b>Browse</b>, select your mount point\n" +
"For <b>Mount Point</b>, enter the path in the local " \
"file system where the directory should be mounted. With\n" +
Copy link
Member

Choose a reason for hiding this comment

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

One more trailing +...

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed. It seems to be a Rubocop bug, already fixed: rubocop/rubocop#1474

@lslezak
Copy link
Member

lslezak commented Dec 2, 2014

I like the idea of separation common settings to an extra file (.rubocop_yast_style.yml) which can be shared across all Yast repositories.

I have some more tips for the shared part (taken from .rubocop.yml in the registration module):

# align arrows:
# "foo"     => true
# "foo_bar" => false
Style/AlignHash:
  EnforcedHashRocketStyle: table

# no extra indentation for multiline function calls
Style/AlignParameters:
  EnforcedStyle: with_fixed_indentation

# no extra indentation for case
Style/CaseIndentation:
  IndentWhenRelativeTo: end

# disabled, unless should be used only in trailing form
Style/NegatedIf:
  Enabled: false

# use "raise" instead of "fail"
Style/SignalException:
  EnforcedStyle: only_raise

@lslezak
Copy link
Member

lslezak commented Dec 16, 2014

LGTM

@mvidner
Copy link
Member Author

mvidner commented Dec 17, 2014

Ah, .changes missing, and I want to retarget this to SLE-12-GA.

@mvidner mvidner mentioned this pull request Jan 8, 2015
@mvidner
Copy link
Member Author

mvidner commented Jan 8, 2015

Retargeting, which needs a new PR.

@mvidner mvidner closed this Jan 8, 2015
@mvidner mvidner deleted the rubocop branch January 8, 2015 09:25
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