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

Enable perlcritic checks in CI #37

Merged
merged 1 commit into from Feb 13, 2024
Merged

Conversation

b10n1k
Copy link
Contributor

@b10n1k b10n1k commented Feb 12, 2024

Run perlcritic against all the perl git files.

https://progress.opensuse.org/issues/138416

Copy link
Member

@kalikiana kalikiana left a comment

Choose a reason for hiding this comment

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

Nice to see that this already flagged two files

tests/boot.pm Outdated
@@ -3,6 +3,7 @@

use base 'basetest';
use strict;
use warnings;
Copy link
Member

Choose a reason for hiding this comment

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

We already established that we don't want to have the explicit use strict and use warnings in test modules. I guess you can find a related discussion already in other pull requests in this repo. Please see http://open.qa/docs/#_test_module_interface for the suggested style. So basically replace the above three lines with

use Mojo::Base 'basetest';

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i know but i found it as that and i didnt want to interfere to the approach. i will change them

- name: Static analysis
run: |
git config --global --add safe.directory '*'
./external/os-autoinst-common/tools/perlcritic --quiet $(git ls-files -- '*.p[ml]')
Copy link
Contributor

Choose a reason for hiding this comment

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

As mentioned in the other PR, please use perlcritic like this:
https://github.com/os-autoinst/os-autoinst-common/blob/master/.github/workflows/perl-critic.yml
You don't need to pass it the list of files.

- uses: actions/checkout@v4
- name: Static analysis
run: |
git config --global --add safe.directory '*'
Copy link
Contributor

Choose a reason for hiding this comment

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

now there is no need to do this anymore, as no git command is called here.

Suggested change
git config --global --add safe.directory '*'

main.pm Outdated
@@ -1,7 +1,8 @@
# Copyright 2014-2018 SUSE LLC
# SPDX-License-Identifier: GPL-2.0-or-later

use strict;

use Mojo::Base;
Copy link
Member

Choose a reason for hiding this comment

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

I assume this should be

Suggested change
use Mojo::Base;
use Mojo::Base -strict;

As no base class is used here

Run perlcritic against all the perl git files.

https://progress.opensuse.org/issues/138416
Signed-off-by: ybonatakis <ybonatakis@suse.com>
@okurz okurz merged commit 1c1a476 into os-autoinst:main Feb 13, 2024
9 checks passed
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.

None yet

4 participants