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

Fix and test perlcritic plugins #54

Merged
merged 5 commits into from
Aug 13, 2024

Conversation

perlpunk
Copy link
Collaborator

@perlpunk perlpunk commented Aug 12, 2024

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

@perlpunk perlpunk force-pushed the perlcritic-plugins branch from 36b17d3 to faa3429 Compare August 12, 2024 14:44
@perlpunk perlpunk force-pushed the perlcritic-plugins branch from faa3429 to 4fcb716 Compare August 13, 2024 10:01
Copy link
Member

@okurz okurz left a comment

Choose a reason for hiding this comment

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

+1

@perlpunk perlpunk marked this pull request as ready for review August 13, 2024 10:03
@kalikiana
Copy link
Member

@perlpunk perlpunk force-pushed the perlcritic-plugins branch from 4fcb716 to 0c2190a Compare August 13, 2024 10:25
.github/workflows/test.yaml Show resolved Hide resolved
@perlpunk perlpunk force-pushed the perlcritic-plugins branch from 0c2190a to 3968962 Compare August 13, 2024 11:16
@mergify mergify bot merged commit 26141c0 into os-autoinst:master Aug 13, 2024
8 checks passed
Comment on lines +71 to +72
my $nsib = $elem->snext_sibling;
my $psib = $elem->sprevious_sibling;
Copy link
Contributor

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?

Copy link
Collaborator Author

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]$/;
Copy link
Contributor

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

maybe...

@b10n1k
Copy link
Contributor

b10n1k commented Aug 13, 2024

I was too late. But if you dont mind take a look on my comments

@perlpunk perlpunk deleted the perlcritic-plugins branch August 14, 2024 13:11
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.

4 participants