-
Notifications
You must be signed in to change notification settings - Fork 9
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
Fix and test perlcritic plugins #54
Conversation
36b17d3
to
faa3429
Compare
faa3429
to
4fcb716
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
4fcb716
to
0c2190a
Compare
0c2190a
to
3968962
Compare
my $nsib = $elem->snext_sibling; | ||
my $psib = $elem->sprevious_sibling; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are those two used somewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch!
@@ -77,7 +79,8 @@ sub _is_block ($token) { | |||
} | |||
|
|||
sub _is_only_one_space ($token) { | |||
return $token->isa('PPI::Token::Whitespace') && $token->content eq ' '; | |||
# We also allow line breaks, e.g. perltidy forces to break up long subroutine headers | |||
return $token->isa('PPI::Token::Whitespace') && $token->content =~ m/^[ \n]$/; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a better function name? Maybe is_whitespace_or_newline
? but not sure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe...
I was too late. But if you dont mind take a look on my comments |
This adds proper unit tests to our perlcritic plugins and fixes some of them.
It also adds the
OpenQA
namespace to the plugin names.I ran it on openQA and os-autoinst now.
For RedundantStrictWarning we will need a couple of
## no critic comments
. there's no way to disable a plugin from the config file apparently. Reducing the severity will still exit with non-zero.Issue: https://progress.opensuse.org/issues/155188