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

Introduce nested limit checks #7519

Merged
merged 1 commit into from May 27, 2019
Merged

Introduce nested limit checks #7519

merged 1 commit into from May 27, 2019

Conversation

b10n1k
Copy link
Contributor

@b10n1k b10n1k commented May 22, 2019

Update .perlcriticrc to run nested limit checks and fixed issue for lib/registration.pm

@b10n1k
Copy link
Contributor Author

b10n1k commented May 23, 2019

the Travis now is failing because of the introduced nested limits.

@b10n1k b10n1k force-pushed the poo48605 branch 3 times, most recently from a3fde16 to 9456c2a Compare May 23, 2019 07:46
Copy link
Member

@rwx788 rwx788 left a comment

Choose a reason for hiding this comment

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

It actually fails with following errors:
tests/migration/sle12_online_migration/yast2_migration.pm: Code structure is deeply nested at line 159, column 21. Consider refactoring. (Severity: 4)
tests/slenkins/slenkins_control.pm: Code structure is deeply nested at line 84, column 21. Consider refactoring. (Severity: 4)

@b10n1k
Copy link
Contributor Author

b10n1k commented May 23, 2019

those are modules that should be assigned to the corresponding teams to take care of. i didnt make anything with them. @rwx788 do you want me to fix those?

@okurz
Copy link
Member

okurz commented May 23, 2019

Hi @b10n1k, I guess fixing these issues yourself is actually easier than waiting for someone else. For the "slenkins" file I am sure there is no other "team" currently that would feel better responsible. However, there would also be the alternative to exclude these two files explicitly from the checks. That sounds a bit messy though ;)

@b10n1k b10n1k force-pushed the poo48605 branch 7 times, most recently from 11aa4ce to 5352bb3 Compare May 27, 2019 08:54

[ControlStructures::ProhibitDeepNests]
severity = 4
add_themes = freenode
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch here, according to documentation, add_themes makes more sense than set_themes even we could see same output.

@jknphy
Copy link
Contributor

jknphy commented May 27, 2019

LGTM

@rwx788 rwx788 merged commit 05ab508 into os-autoinst:master May 27, 2019
@b10n1k b10n1k deleted the poo48605 branch August 24, 2023 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants