style: Always pass a regex match to split

[perlpunk]

A quoted string works as well for literal strings, but it is inconsistent

Can be enforced later by BuiltinFunctions::ProhibitStringySplit or Community::SplitQuotedPattern

See also

• RFC: Include core perlcritic rules as well #7056

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.72%. Comparing base (ead9b8b) to head (403f4f7).
⚠️ Report is 6 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #7082   +/-   ##
=======================================
  Coverage   99.72%   99.72%           
=======================================
  Files         416      416           
  Lines       43076    43076           
=======================================
  Hits        42956    42956           
  Misses        120      120           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[perlpunk]

:(
https://github.com/os-autoinst/openQA/actions/runs/22732763046/job/65925969913?pr=7082

#4 [webui internal] load metadata for docker.io/opensuse/leap:16.0
#4 ERROR: failed to copy: httpReadSeeker: failed open: unexpected status code
 https://registry-1.docker.io/v2/opensuse/leap/manifests/sha256:859560554b625c225fa767b76d61253d529b95d082c2d68579ad69168d5e3da7:
 429 Too Many Requests - Server message: toomanyrequests:
 You have reached your unauthenticated pull rate limit. https://www.docker.com/increase-rate-limit

[okurz]

I would be interested how the actual automatic rule enforcement would be configured. I don't see this style as strictly necessary but I favor consistency and adherence to standards. So, you are stating

Can be enforced later by BuiltinFunctions::ProhibitStringySplit or Community::SplitQuotedPattern
but why not enable such rules directly? Where we configure those? ANd you have a URL to an external reference that states "this is a good standard"?

[perlpunk]

I would be interested how the actual automatic rule enforcement would be configured.

I can think of two ways.

The current content of https://github.com/os-autoinst/openQA/blob/master/external/os-autoinst-common/.perlcriticrc is:

theme = community + openqa
severity = 4
include = strict ValuesAndExpressions::ProhibitInterpolationOfLiterals
...

We can either
a) add theme = community + openqa + core and then add an exclude line with all the rules that don't make sense for us
b) Add the rules we want to use to the include line

I think b) would make most sense for now as we add rules. If we use the majority of rules, then exclude might make more sense, but I doubt it.

As I stated in the other PR, I did not directly change .perlcriticrc as part of this PR, because it is a symlink to the .perlcriticrc in the subrepo os-autoinst-common, so we should not change that directly, but change it in os-autoinst-common first and then pull the subrepo.
But if I create the PR in os-autoinst-common first, we cannot see how the actual change looks like.
And there are a lot of other rules we might or might not want, so I think it makes more sense if I do the changes first, we can decide if we want them and then at some point make a PR to the config in os-autoinst-common.
I don't want to do the whole subrepo process for every single rule.
You can see the current rule violations here: https://app.circleci.com/pipelines/github/os-autoinst/openQA/19719/workflows/0a9a1769-4ac0-4459-abb6-392e2fd27a29/jobs/196751 as part of the RFC #7056

Does anything speak against this? Of course if we not enforce it immediately, there might be code added meanwhile that does not adhere to one of the new rules. But my goal is to change most of the lines now, and when we enforce them later, there can be a handful of lines that we have to change, but not 500 like in my first PR.

but why not enable such rules directly?

What we could also do: Temporarily make the .perlcriticrc a real file and not a symlink, until we settled on a set of rules, and then make the change in os-autoinst-common and symlink it again.

I don't see this style as strictly necessary but I favor consistency and adherence to standards. So, you are stating

Can be enforced later by BuiltinFunctions::ProhibitStringySplit or Community::SplitQuotedPattern

Where we configure those?

I think the quoting was broken, so I fixed it, so the "but why" was from you.
Like I said, it's configured in .perlcriticrc as described above.
#7056 shows how it would look like if we use the exclude approach.

ANd you have a URL to an external reference that states "this is a good standard"?

Perl::Critic was inspired after the book Perl Best Practices from Damian Conway. Who himself also pointed out that the rules in the book are opinionated, and the most important thing is that you find good reasons for or against a rule, but be consistent.

[perlpunk]

I don't see this style as strictly necessary but I favor consistency and adherence to standards.

So would you actually vote for this rule or against it?

[Martchus]

Will using a regex be slower?

[perlpunk]

Will using a regex be slower?

No. It always uses a regex, just that it accepts it to be passed as a string as well.