style: Use builtin functions without parentheses consistently

[perlpunk]

This can be ensured later with a perlcritic rule.

There is no perlcritic rule that I know of that can ensure the opposite, so we can only choose to ensure no parentheses, or leave it inconsistently.

I personally find it more readable without parens.
There are some cases where it is or can seem ambiguous, or where it would do the wrong thing, so I suggest to use parentheses around the fuction call instead:

my $s = 'foo ' . (join ':', $x, $y);

# here it wouldn't be necessary if the `join` is the last thing in the list, but as soon as you add things after it
my %params = (x => 23, string => (join ':', $x, $y));

See also

• RFC: Include core perlcritic rules as well #7056

Let me know what you think.

[perlpunk]

We could ensure this already now by adding CodeLayout::ProhibitParensWithBuiltins to the include line in .perlcriticrc, but this file is a symlink to os-autoinst-common, so we should change it there instead, after we decided to use it.

[codecov]

Codecov Report

❌ Patch coverage is 98.67374% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 99.72%. Comparing base (2bd9265) to head (38b8014).
⚠️ Report is 6 commits behind head on master.

Files with missing lines	Patch %	Lines
lib/OpenQA/CacheService/Controller/Influxdb.pm	0.00%	1 Missing ⚠️
lib/OpenQA/Schema/Result/JobLocks.pm	0.00%	1 Missing ⚠️
lib/OpenQA/Schema/Result/Jobs.pm	96.42%	1 Missing ⚠️
lib/OpenQA/Schema/Result/ScheduledProducts.pm	93.33%	1 Missing ⚠️
lib/OpenQA/Script/CloneJob.pm	75.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #7076   +/-   ##
=======================================
  Coverage   99.72%   99.72%           
=======================================
  Files         416      416           
  Lines       42979    42979           
=======================================
  Hits        42859    42859           
  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]

Added not-ready to give more people the chance to review

[perlpunk]

Should I leave out the 5 uncovered lines for now?

[okurz]

Should I leave out the 5 uncovered lines for now?

I guess we want to enable strict enforced rules so we should use the changes everywhere. At best we provide test coverage for those missing lines as after all we are "nearly there" to have everything covered. Alternatively we force-merge with missing coverage as we can trust that no functional changes are introduced if we focus on changing the style only.

[perlpunk]

Alternatively we force-merge with missing coverage as we can trust that no functional changes are introduced if we focus on changing the style only.

To me the lines look harmless enough to force-merge. But we could also change them back and mark them with no critic.

But in general I think it would be good to merge this as soon as we voted for it.

[perlpunk]

No objections from anyone. Merging manually - the patched lines don't look like they could break anything