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
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
5a9e135
Generated a Rubocop config
mvidner Nov 26, 2014
ce8b5b3
rubocop: SUSE style is double quotes
mvidner Nov 26, 2014
3fe2277
rubocop: hash_rockets as produced by YCP Killer
mvidner Nov 26, 2014
d677058
rubocop: fixed Style/Lambda
mvidner Nov 26, 2014
7f71f3b
rubocop: Style/MethodName: snake_case, exclude existing violations
mvidner Nov 26, 2014
d20263b
rubocop: Metrics/LineLength 135 -> 116
mvidner Nov 26, 2014
46063fb
rubocop: Style/VariableName: snake_case, except in old tests
mvidner Nov 26, 2014
fae8099
rubocop: Style/LineEndConcatenation
mvidner Nov 26, 2014
2793dc0
rubocop: Style/Documentation, except old tests
mvidner Nov 26, 2014
6d01188
rubocop: Style/LeadingCommentSpace
mvidner Nov 26, 2014
b5820ed
rubocop: Style/WordArray is disabled per SUSE style
mvidner Nov 26, 2014
dbe113c
rubocop: Lint/UselessAssignment
mvidner Nov 26, 2014
2abfe37
rubocop: Lint/Loop
mvidner Nov 26, 2014
097355d
rubocop: Lint/LiteralInCondition
mvidner Nov 26, 2014
342679e
rubocop: Lint/UnusedMethodArgument
mvidner Nov 26, 2014
b16c5e1
rubocop: Style/BlockEndNewline:
mvidner Nov 26, 2014
9f9f76a
rubocop: Style/BracesAroundHashParameters
mvidner Nov 26, 2014
30e0dc2
rubocop: Style/CommentAnnotation
mvidner Nov 26, 2014
609425e
rubocop: Style/EmptyLines Style/EmptyLinesAroundBody
mvidner Nov 26, 2014
f74432a
rubocop: Style/IfUnlessModifier
mvidner Nov 26, 2014
ff2e9fc
rubocop: Style/IndentationConsistency Style/IndentationWidth
mvidner Nov 26, 2014
4acbc75
rubocop: Style/InfiniteLoop
mvidner Nov 26, 2014
eed1c5b
rubocop: Style/FileName, renamed tests, excluded existing sources
mvidner Nov 26, 2014
f326b9b
rubocop: Style/MultilineBlockLayout
mvidner Nov 26, 2014
9023c7f
rubocop: Style/MultilineTernaryOperator
mvidner Nov 26, 2014
29a3e8d
rubocop: Style/NilComparison
mvidner Nov 26, 2014
18bed3e
rubocop: Style/NonNilCheck: IncludeSemanticChanges
mvidner Nov 26, 2014
6c98b4c
rubocop: Style/SpaceAfterNot Style/SpaceBeforeBlockBraces
mvidner Nov 26, 2014
f380d9e
rubocop: Style/TrailingComma
mvidner Nov 26, 2014
d4336bb
rubocop: Style/TrailingWhitespace
mvidner Nov 26, 2014
85fbad6
rubocop: Style/TrivialAccessors disabled inline
mvidner Nov 26, 2014
4183ff5
rubocop: remove config that is no longer needed
mvidner Nov 26, 2014
50596ad
rubocop: updated metrics, slightly stricter
mvidner Nov 26, 2014
0121473
rubocop: config updated for 0.27.1.
mvidner Nov 26, 2014
8cec1eb
rubocop: enable in Travis
mvidner Nov 26, 2014
8c32bf3
rubocop: rearranged the config after review
mvidner Dec 1, 2014
7957ec1
better type info in doc comments
mvidner Dec 1, 2014
183b843
Fixed a condition that was always true.
mvidner Dec 1, 2014
5351b49
rubocop: Style/LineEndConcatenation
mvidner Dec 10, 2014
ee4a3c0
rubocop: more common style (from yast2-registration)
mvidner Dec 10, 2014
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
inherit_from:
.rubocop_yast_style.yml

# Offense count: 34
Metrics/AbcSize:
Max: 220

# Offense count: 5
# Configuration parameters: CountComments.
Metrics/ClassLength:
Max: 513

# Offense count: 13
Metrics/CyclomaticComplexity:
Max: 31

# Offense count: 49
# Configuration parameters: AllowURI, URISchemes.
Metrics/LineLength:
Max: 116

# Offense count: 47
# Configuration parameters: CountComments.
Metrics/MethodLength:
Max: 223

# Offense count: 13
Metrics/PerceivedComplexity:
Max: 35

Style/Documentation:
Exclude:
- testsuite/tests/*.rb

Style/FileName:
Exclude:
- src/clients/nfs-client.rb
- src/clients/nfs-client4part.rb
- src/modules/Nfs.rb
- src/modules/NfsOptions.rb

# YCP Killer used rockets
Style/HashSyntax:
Exclude:
- src/clients/nfs.rb
- src/clients/nfs_auto.rb
- src/include/nfs/ui.rb
- src/include/nfs/wizards.rb
- src/modules/Nfs.rb
- src/modules/NfsOptions.rb

Style/MethodName:
Exclude:
- src/clients/nfs-client4part.rb
- src/clients/nfs.rb
- src/include/nfs/routines.rb
- src/include/nfs/ui.rb
- src/include/nfs/wizards.rb
- src/modules/Nfs.rb

Style/VariableName:
Exclude:
- testsuite/tests/*.rb
41 changes: 41 additions & 0 deletions .rubocop_yast_style.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# Rubocop style configuration
#
# Following
# https://github.com/SUSE/style-guides/blob/master/Ruby.md

# https://github.com/SUSE/style-guides/blob/master/Ruby.md#strings
Style/StringLiterals:
EnforcedStyle: double_quotes

Style/StringLiteralsInInterpolation:
EnforcedStyle: double_quotes

# Is there any justification for "aligned" which is the default?
Style/MultilineOperationIndentation:
EnforcedStyle: indented

# https://github.com/SUSE/style-guides/blob/master/Ruby.md#arrays
Style/WordArray:
Enabled: false
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 general for generic yast style should be also:

# do not force %r
Style/RegexpLiteral:
 Enabled: false

Style/SignalException:
 EnforcedStyle: only_raise

Style/CaseIndentation:
 IndentWhenRelativeTo: end

# Do not allow and or or in conditionals
Style/AndOr:
 EnforcedStyle: conditionals

# ycp killer use aligned hash, so I think we can continue with its usage as it is really much better, especially in modules which heavily use hashes
Style/AlignHash:
 EnforcedHashRocketStyle: table
 EnforcedColonStyle: table

# We need to decide and this is easier to recognize in code
Style/AccessModifierIndentation:
 EnforcedStyle: outdent

Just parts from bootloader config, that I find useful and helpful.


# 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

# "unless" has a different connotation than "if not"
Style/NegatedIf:
Enabled: false

# use "raise" instead of "fail"
Style/SignalException:
EnforcedStyle: only_raise
3 changes: 2 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ before_install:
# disable rvm, use system Ruby
- rvm reset
- wget https://raw.githubusercontent.com/yast/yast-devtools/master/travis-tools/travis_setup.sh
- sh ./travis_setup.sh -p "rake yast2-devtools yast2-testsuite yast2" -g "rspec:2.14.1 yast-rake gettext"
- sh ./travis_setup.sh -p "rake yast2-devtools yast2-testsuite yast2" -g "rspec:2.14.1 yast-rake gettext rubocop"
script:
- rake check:syntax
- rake check:pot
- rubocop
- make -f Makefile.cvs
- make
- sudo make install
Expand Down
2 changes: 1 addition & 1 deletion Rakefile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
require "yast/rake"

Yast::Tasks.configuration do |conf|
#lets ignore license check for now
# lets ignore license check for now
conf.skip_license_check << /.*/
end
5 changes: 2 additions & 3 deletions src/clients/nfs-client.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
# encoding: utf-8

# Author: Martin Vidner <mvidner@suse.cz>
# Summary: Just a redirection
# $Id$
# YaST namespace
module Yast
# Just a redirection
class NfsClientClient < Client
def main
@target = "nfs"
Expand Down
8 changes: 3 additions & 5 deletions src/clients/nfs-client4part.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
# encoding: utf-8

# File: clients/
# Summary: nfs-client stuff made accesible from
# Author: Bubli <kmachalkova@suse.cz>
#
# YaST namespace
Copy link
Member

Choose a reason for hiding this comment

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

does it make sense to have this duplicite useless comment everywhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

I see it as a small cost to keep Style/Documentation enabled, which makes me summarize all classes.

Copy link
Member

Choose a reason for hiding this comment

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

Ugh? Bootloader pass it and it do not have such stuff. I am sure, that document it on only one place make rubocop happy.

Copy link
Member Author

Choose a reason for hiding this comment

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

Documenting in one place does not make Rubocop happy. The cop description, "Document classes and non-namespace modules.", hints at the difference: it complains when the module is not a pure namespace, containing the YaST initialization: Foo = FooClass.new; Foo.main. Your new code does not have that.

Copy link
Member

Choose a reason for hiding this comment

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

OK, it make sense. So in fact it is wrong detection that it is pure namespace as it actually is.

module Yast
# nfs-client stuff made accesible from the partitioner
class NfsClient4partClient < Client
def main
Yast.import "UI"
Expand Down Expand Up @@ -76,7 +74,7 @@ def ToStorage(entry)
entry = deep_copy(entry)
ret = {}

if entry != nil && entry != {}
if entry && entry != {}
ret = {
"device" => Ops.get_string(entry, "spec", ""),
"mount" => Ops.get_string(entry, "file", ""),
Expand Down
26 changes: 6 additions & 20 deletions src/clients/nfs.rb
Original file line number Diff line number Diff line change
@@ -1,27 +1,14 @@
# encoding: utf-8

# File: clients/nfs.ycp
# Package: Configuration of NFS
# Summary: Main file
# Authors:
# Jan Holesovsky <kendy@suse.cz>
# Dan Vesely <dan@suse.cz>
# Martin Vidner <mvidner@suse.cz>
#
# $Id$
#
# Main file for nfs configuration. Uses all other files.
# YaST namespace
module Yast
# Main file
class NfsClient < Client
def main
Yast.import "UI"

#**
# <h3>Configuration of NFS</h3>

textdomain "nfs"

# The main ()
Builtins.y2milestone("----------------------------------------")
Builtins.y2milestone("NFS module started")

Expand All @@ -47,7 +34,7 @@ def main
"finish" => fun_ref(Nfs.method(:Write), "boolean ()"),
"actions" => {
"list" => {
# TODO summary is probably better...
# TODO: summary is probably better...
"handler" => fun_ref(
method(:NfsListHandler),
"boolean (map)"
Expand All @@ -74,7 +61,7 @@ def main
}
},
"options" => {
# TODO adjust names? create comaptibility aliases?
# TODO: adjust names? create comaptibility aliases?
"spec" => {
"type" => "string",
# host:path
Expand Down Expand Up @@ -138,7 +125,7 @@ def main
Builtins.y2milestone("NFS module finished")
Builtins.y2milestone("----------------------------------------")

deep_copy(@ret)
deep_copy(@ret)

# EOF
end
Expand All @@ -147,8 +134,7 @@ def main
# Print summary in command line
# @param [Hash] options command options
# @return false so that Write is not called in non-interactive mode
def NfsListHandler(options)
options = deep_copy(options)
def NfsListHandler(_options)
nfs_entries = deep_copy(Nfs.nfs_entries)
if Ops.less_than(Builtins.size(nfs_entries), 1)
CommandLine.Print(Summary.NotConfigured)
Expand Down
29 changes: 5 additions & 24 deletions src/clients/nfs_auto.rb
Original file line number Diff line number Diff line change
@@ -1,28 +1,8 @@
# encoding: utf-8

# File:
# nfs_auto.ycp
#
# Package:
# Configuration of NFS client
#
# Summary:
# Client for autoinstallation
#
# Authors:
# Martin Vidner <mvidner@suse.cz>
#
# $Id$
#
# This is a client for autoinstallation. It takes its arguments,
# goes through the configuration and return the setting.
# Does not do any changes to the configuration.

# @param first a map of mail settings
# @return [Hash] edited settings or empty map if canceled
# @example map mm = $[ "FAIL_DELAY" : "77" ];
# @example map ret = WFM::CallModule ("mail_auto", [ mm ]);
# YaST namespace
module Yast
# Client for autoinstallation
class NfsAutoClient < Client
def main
Yast.import "UI"
Expand Down Expand Up @@ -54,7 +34,8 @@ def main
if Ops.greater_than(Builtins.size(WFM.Args), 1) &&
Ops.is_list?(WFM.Args(1))
Builtins.y2warning(
"Old-style configuration detected (got list, expected map). <nfs> section needs to be converted to match up-to-date schema"
"Old-style configuration detected (got list, expected map). " \
"<nfs> section needs to be converted to match up-to-date schema"
)
end
end
Expand Down Expand Up @@ -104,7 +85,7 @@ def main
Builtins.y2milestone("Nfs auto finished")
Builtins.y2milestone("----------------------------------------")

deep_copy(@ret)
deep_copy(@ret)

# EOF
end
Expand Down
41 changes: 12 additions & 29 deletions src/include/nfs/routines.rb
Original file line number Diff line number Diff line change
@@ -1,25 +1,10 @@
# encoding: utf-8

# File:
# routines.ycp
#
# Module:
# Configuration of nfs
#
# Summary:
# Network NFS routines
#
# Authors:
# Jan Holesovsky <kendy@suse.cz>
# Dan Vesely <dan@suse.cz>
#
# $Id$
#
# Network NFS routines
#
# YaST namespace
module Yast
# Miscellaneous
module NfsRoutinesInclude
def initialize_nfs_routines(include_target)
def initialize_nfs_routines(_include_target)
textdomain "nfs"

Yast.import "Package"
Expand All @@ -37,7 +22,7 @@ def SpecToServPath(spec)
serv = ""

# no :/ inside => <server>: or [/]<path>
if path_begin == nil
if path_begin.nil?
Copy link
Member

Choose a reason for hiding this comment

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

you can use if !path_begin

Copy link
Member Author

Choose a reason for hiding this comment

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

But only if path_begin cannot be false. That is a different kind of change than I want to do in this PR.

if spec ==
Ops.add(
Builtins.filterchars(spec, Ops.add("-_.", String.CAlnum)),
Expand All @@ -48,14 +33,13 @@ def SpecToServPath(spec)
end
end

if path_begin != nil
if path_begin
serv = Builtins.substring(spec, 0, path_begin)
spec = Builtins.substring(spec, Ops.add(path_begin, 1))
end
term(:couple, serv, spec)
end


# Creates a list of ui table items for nfs fstab entries
# @param [Array<Hash>] fstab list of nfs fstab entries
# @return itemized table entries
Expand Down Expand Up @@ -97,11 +81,11 @@ def CheckHostName(name)
Report.Error(
Builtins.sformat(
_(
"The hostname entered is invalid. It must be\n" +
"shorter than 50 characters and only use\n" +
"valid IPv4, IPv6 or domain name.\n" +
"Valid IPv4: %1\n" +
"Valid IPv6: %2\n" +
"The hostname entered is invalid. It must be\n" \
"shorter than 50 characters and only use\n" \
"valid IPv4, IPv6 or domain name.\n" \
"Valid IPv4: %1\n" \
"Valid IPv6: %2\n" \
"Valid domain: %3"
),
IP.Valid4,
Expand Down Expand Up @@ -153,16 +137,15 @@ def CheckPath(name)
Report.Error(
Builtins.sformat(
_(
"The path entered is invalid.\n" +
"It must be shorter than 70 characters\n" +
"The path entered is invalid.\n" \
"It must be shorter than 70 characters\n" \
"and it must begin with a slash (/)."
)
)
)
false
end


# Strips a superfluous slash off the end of a pathname.
# @param [String] p pathname
# @return stripped pathname
Expand Down
Loading