Centralize perl lint checks #30

josegomezr · 2023-10-25T16:03:31Z

Introduced simplified version of tools/perlcritic.

The perlcritic wrapper provided under tools/perlcritic will resolve itself
and append the common perl critic policies defined in this repo automatically
providing a seamless experience.
Introduced simplified version tools/tidyall [warning, is breaking].

This is a full rework of the script to perform the same tidy version detection
in perl instead of deep bash + sed.

It'll validate the tidy version and run tidyall afterwards.

Support for the --force flag is also provided transparently.

The breaking change comes from the fact that the bash script provided a façade
over tidyall transforming arguments. This is deleted in favor of the real
tidyall arguments.

Downstream repositories will inherit this tools through the subrepo pull workflow.

poo#138416

tools/tidyall

.perltidyrc

Testing the perltidy config from os-autoinst/os-autoinst-common#30

tools/tidyall

.circleci/config.yml

okurz

well, see the style warnings that you introduced I woul say :)

.github/workflows/perl-prove.yml

t/01-make-update-deps.t

josegomezr · 2023-11-08T09:48:13Z

I tried experimenting on annotations for failed tests, but the test infrastructure lacks a lot on controlling the output.

Brute-forcing my way with a GH annotation get's me to a not-so-reliable regex, see it here for the regex and a sample output match:

https://regex101.com/r/9HVPpB/1

The furthest I could get was to get which test failed, in: josegomezr/perl-design-patterns: t/lib/TAP/Formatter/GitHubAction.pm).

I'll backtrack on that one until a simpler solution rises, for now it'll just be a red check when tests fail.

lib/perlcritic/Perl/Critic/Policy/HashKeyQuotes.pm

tools/perlcritic

tools/update-deps

xt/01-make-update-deps.t

cpanfile

perlpunk

There are a lot of things I could comment on.
But I think we should discuss how this should be tested and which existing repositories should use which tools already. Not sure what @okurz had in mind.
I think one necessary step for approval is that this is tested on one of our repos. (For example the tools/update-deps doesn't compile with your changes :)

So I would suggest doing a draft PR with this branch as a subrepo:

git subrepo clone --force [email protected]:josegomezr/os-autoinst-common.git external/os-autoinst-common -b unify_perltidy

perlpunk · 2023-11-13T10:38:24Z

.github/workflows/perl-lint-checks.yml

+    runs-on: ubuntu-latest
+    name: "Perltidy"
+    container:
+      image: perldocker/perl-tester:5.26
+    steps:
+      - uses: actions/checkout@v4
+      - run: GITHUB_ACTIONS=1 ./tools/tidyall --check-only --all --quiet


I think with this we would be getting the latest Perl::Tidy that is installed in that container, so people will have to install that version locally. It will not be in sync with https://build.opensuse.org/package/show/devel:openQA:Leap:15.5/perl-Perl-Tidy

Yes, this is still requiring some love, I have a pending experiment making this image (perl-tester) in OBS with openqa dependencies instead. I think I spotted something like that already there.

.gitignore

cpanfile

lib/OpenQA/Test/PatchDeparse.pm

tools/perlcritic

tools/tidyall

tools/update-deps

tools/tidyall

Martchus

I guess the last commit should be squashed into the commit that introduced the code.

README.md

lib/perlcritic/Perl/Critic/Policy/HashKeyQuotes.pm

lkocman · 2023-11-27T13:00:31Z

Our perl interpret is inherited from SUSE:SLE-15-SP3:Update. So the right question is whether sles can update it. mlschroe should be able to tell us, we're ahead of beta2 so still possible imho.

lkocman · 2023-11-27T13:10:21Z

https://jira.suse.com/browse/PED-7392

perlpunk · 2023-11-29T15:26:37Z

tools/update-deps

@@ -39,8 +40,7 @@ update_spec();
 update_cpanfile($modules_by_target);
 update_dockerfile($dockerfile) if $dockerfile;

-sub update_dockerfile {
-    my ($dockerfile) = @_;
+sub update_dockerfile ($dockerfile) {


I just tried this out in os-autoinst. The script doesn't compile because you still need to enable signatures

josegomezr · 2023-12-06T22:26:47Z

Adding a bit more of activity here:

In the PoC on my fork of the example distribution (available here I've:

Brought all applicable configs & tooling from this branch.
Setup the checks (these are copied because github doesn't tolerate symlinks for security reasons).
Fixed the perlcritic warnings.
The OpenQA scheduling job has a safeguard to avoid having the test in red when there's no API keys configured.

Addenda:
Bonus:

Simplified the GitHub Actions workflows for the isotovideo invocation.
- Artifacts! It's possible to download test execution results from the isotovideo invocation.
- And of the repo can customize how to run isotovideo via a matrix config as a means of a simplified scheduling experience.

okurz · 2023-12-07T13:14:18Z

@josegomezr it's getting hard to see where we are going overall. I suggest you split out commits from https://github.com/os-autoinst/os-autoinst-common/pull/30/commits into separate PRs that we can get merged and "out of sight" and then rebase here and see what's left.

- Introducing `tools/tidyall` an improved wrapper over perl's `tidyall`. It does version detection directly in perl and exposes the underlying `tidyall` CLI. - Adds a complementary GitHub Action to run `tools/tidyall` automatically on Pull Requests & Master. - Brings bare minimum dependencies.yaml for running `tools/tidyall`. Adjusted `tools/update-deps` to produce a `cpanfile` alone (without needing an `.spec` file first). - Applied tidy rules. Branched off os-autoinst#30.

- Introducing `tools/perlcritic` an improved wrapper over perl's `perlcritic`. It automatically appends this project's policies that are defined under the `openqa` theme. - Adds a complementary GitHub Action to run `tools/perlcritic` automatically on Pull Requests & Master. - Fixed `perlcritics` complaints. Branched off os-autoinst#30.

josegomezr · 2023-12-14T12:24:35Z

@josegomezr it's getting hard to see where we are going overall. I suggest you split out commits from https://github.com/os-autoinst/os-autoinst-common/pull/30/commits into separate PRs that we can get merged and "out of sight" and then rebase here and see what's left.

#31 & #32 branch off this discussion. Please have a look, they can be merged in isolation. And after that I'll get back this one and finish-off any pending piece.

- Introducing `tools/perlcritic` an improved wrapper over perl's `perlcritic`. It automatically appends this project's policies that are defined under the `openqa` theme. - Adds a complementary GitHub Action to run `tools/perlcritic` automatically on Pull Requests & Master. - Fixed `perlcritics` complaints. Branched off os-autoinst#30. Co-authored-by: Oliver Kurz <[email protected]> Co-authored-by: Martchus <[email protected]>

foursixnine · 2024-01-12T18:24:35Z

@okurz what is needed for this PR to be merged, aside from resolving the coflicts, etc?. I see that there's at least one discussion open, also because looking at the ticket, looks like we're waiting on @os-autoinst/tools-team for all of the related PRs.

And since @josegomezr is back to SCC, I would like to know who is going to take over these changes, since it has taken 3 months, and still can't be closed.

PS: Before @okurz comes saying again that "I'm asking the team to work faster", the answer is no, but it looks that you prefer to make work more complicated that it should, and being in emergency mode all the time doesn't really help.

okurz · 2024-01-12T20:08:57Z

@okurz what is needed for this PR to be merged, aside from resolving the coflicts, etc?. I see that there's at least one discussion open, also because looking at the ticket, looks like we're waiting on @os-autoinst/tools-team for all of the related PRs.

I don't really know what's needed as there had been already quite some changes in the related PRs, e.g. #31, so I suggest to rebase first.

And since @josegomezr is back to SCC, I would like to know who is going to take over these changes, since it has taken 3 months, and still can't be closed.

It might fit probably somewhere with https://progress.opensuse.org/issues/108527 but it's better to make that explicit by taking over https://progress.opensuse.org/issues/138416 into the backlog of qe-tools, agreed?

foursixnine · 2024-01-12T21:45:14Z

@okurz what is needed for this PR to be merged, aside from resolving the coflicts, etc?. I see that there's at least one discussion open, also because looking at the ticket, looks like we're waiting on @os-autoinst/tools-team for all of the related PRs.

I don't really know what's needed as there had been already quite some changes in the related PRs, e.g. #31, so I suggest to rebase first.

And since @josegomezr is back to SCC, I would like to know who is going to take over these changes, since it has taken 3 months, and still can't be closed.

It might fit probably somewhere with https://progress.opensuse.org/issues/108527 but it's better to make that explicit by taking over https://progress.opensuse.org/issues/138416 into the backlog of qe-tools, agreed?

Sounds like a plan! We can have a chat about the things that are related kind of soonish, and at some other point dig a bit more into https://progress.opensuse.org/issues/108527

josegomezr · 2024-01-14T09:56:09Z

You'all can ask me too, I moved team not organizations...

#32 is missing. after it's merged what's pending is:

Wording on documentation.
Tests on update-deps provided that this very same repo requries a very limited subset of packages (lint rules).

That should serve a good baseline to continue downstreaming the tools in #31 & #32 to os-autoinst & openqa.

psa: I'm very bad with technology...

mergify · 2024-01-14T13:54:32Z

This pull request is now in conflicts. Could you fix it? 🙏

- Introducing `tools/perlcritic` an improved wrapper over perl's `perlcritic`. It automatically appends this project's policies that are defined under the `openqa` theme. - Adds a complementary GitHub Action to run `tools/perlcritic` automatically on Pull Requests & Master. - Fixed `perlcritics` complaints. Branched off os-autoinst#30. Co-authored-by: Oliver Kurz <[email protected]> Co-authored-by: Martchus <[email protected]>

okurz · 2024-01-15T09:13:48Z

With

Unify Perl Critic #32

merged can you now rebase here to fix the conflicts?

josegomezr force-pushed the unify_perltidy branch 2 times, most recently from 1823dc9 to 4112c8d Compare October 25, 2023 20:11

perlpunk reviewed Oct 25, 2023

View reviewed changes

tools/tidyall Show resolved Hide resolved

perlpunk reviewed Oct 25, 2023

View reviewed changes

.perltidyrc Show resolved Hide resolved

josegomezr added a commit to josegomezr/openQA that referenced this pull request Oct 25, 2023

experiment: Unify perl linting rules

c3d8fe8

Testing the perltidy config from os-autoinst/os-autoinst-common#30

josegomezr added a commit to josegomezr/os-autoinst that referenced this pull request Oct 25, 2023

experiment: Unify perl linting rules

2db8105

Testing the perltidy config from os-autoinst/os-autoinst-common#30

josegomezr mentioned this pull request Oct 25, 2023

Unify perl linting rules os-autoinst/os-autoinst#2399

Merged

josegomezr added a commit to josegomezr/os-autoinst that referenced this pull request Oct 25, 2023

experiment: Unify perl linting rules

2afb471

Testing the perltidy config from os-autoinst/os-autoinst-common#30

josegomezr mentioned this pull request Oct 25, 2023

Unify perl linting rules os-autoinst/openQA#5348

Open

josegomezr added a commit to josegomezr/os-autoinst that referenced this pull request Oct 25, 2023

experiment: Unify perl linting rules

afdf840

Testing the perltidy config from os-autoinst/os-autoinst-common#30

josegomezr added a commit to josegomezr/os-autoinst that referenced this pull request Oct 25, 2023

experiment: Unify perl linting rules

5546703

Testing the perltidy config from os-autoinst/os-autoinst-common#30

perlpunk reviewed Oct 25, 2023

View reviewed changes

tools/tidyall Outdated Show resolved Hide resolved

josegomezr force-pushed the unify_perltidy branch from 1d8fb0e to f7f3cc9 Compare October 26, 2023 07:48

perlpunk reviewed Oct 26, 2023

View reviewed changes

.circleci/config.yml Outdated Show resolved Hide resolved

josegomezr force-pushed the unify_perltidy branch 4 times, most recently from 0a8ca65 to bb7290a Compare October 26, 2023 16:41

okurz requested changes Nov 7, 2023

View reviewed changes

.github/workflows/perl-prove.yml Outdated Show resolved Hide resolved

t/01-make-update-deps.t Outdated Show resolved Hide resolved

okurz requested changes Nov 8, 2023

View reviewed changes

lib/perlcritic/Perl/Critic/Policy/HashKeyQuotes.pm Show resolved Hide resolved

tools/perlcritic Show resolved Hide resolved

tools/update-deps Show resolved Hide resolved

xt/01-make-update-deps.t Outdated Show resolved Hide resolved

josegomezr force-pushed the unify_perltidy branch 3 times, most recently from 9d3d348 to 36b4039 Compare November 8, 2023 15:23

okurz reviewed Nov 13, 2023

View reviewed changes

cpanfile Outdated Show resolved Hide resolved

perlpunk reviewed Nov 13, 2023

View reviewed changes

tools/tidyall Outdated Show resolved Hide resolved

josegomezr force-pushed the unify_perltidy branch from 1636ce9 to 6417c43 Compare November 13, 2023 12:38

Martchus reviewed Nov 13, 2023

View reviewed changes

README.md Outdated Show resolved Hide resolved

lib/perlcritic/Perl/Critic/Policy/HashKeyQuotes.pm Outdated Show resolved Hide resolved

perlpunk reviewed Nov 29, 2023

View reviewed changes

josegomezr and others added 4 commits November 29, 2023 17:01

Applying styles & aligning signatures in perl code

e386dd6

Adding a note for the Storable explicit dependency

b9afc9a

Make tidyall check for cpanfile in more locations

c805f19

Add missing signatures to tools/update-deps

0fee352

josegomezr force-pushed the unify_perltidy branch from 99ed5c2 to 0fee352 Compare November 29, 2023 16:02

foursixnine approved these changes Dec 14, 2023

View reviewed changes

josegomezr mentioned this pull request Dec 14, 2023

Unify Perl Tidyall #31

Merged

josegomezr mentioned this pull request Dec 14, 2023

Unify Perl Critic #32

Merged

okurz mentioned this pull request Dec 20, 2023

Align CI workflows from os-autoinst-common and isotovideo os-autoinst/os-autoinst-distri-example#28

Merged

Undo quote & whitespaces

29af88e

josegomezr force-pushed the unify_perltidy branch from 656b9b4 to 29af88e Compare January 14, 2024 13:54

perlpunk mentioned this pull request Jan 18, 2024

git-subrepo update external/os-autoinst-common os-autoinst/os-autoinst#2443

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Centralize perl lint checks #30

Centralize perl lint checks #30

josegomezr commented Oct 25, 2023

okurz left a comment

josegomezr commented Nov 8, 2023 •

edited

Loading

perlpunk left a comment

perlpunk Nov 13, 2023

josegomezr Nov 13, 2023

Martchus left a comment

lkocman commented Nov 27, 2023

lkocman commented Nov 27, 2023

perlpunk Nov 29, 2023

josegomezr commented Dec 6, 2023 •

edited

Loading

okurz commented Dec 7, 2023

josegomezr commented Dec 14, 2023

foursixnine commented Jan 12, 2024 •

edited

Loading

okurz commented Jan 12, 2024

foursixnine commented Jan 12, 2024

josegomezr commented Jan 14, 2024 •

edited

Loading

mergify bot commented Jan 14, 2024

okurz commented Jan 15, 2024

Centralize perl lint checks #30

Are you sure you want to change the base?

Centralize perl lint checks #30

Conversation

josegomezr commented Oct 25, 2023

okurz left a comment

Choose a reason for hiding this comment

josegomezr commented Nov 8, 2023 • edited Loading

perlpunk left a comment

Choose a reason for hiding this comment

perlpunk Nov 13, 2023

Choose a reason for hiding this comment

josegomezr Nov 13, 2023

Choose a reason for hiding this comment

Martchus left a comment

Choose a reason for hiding this comment

lkocman commented Nov 27, 2023

lkocman commented Nov 27, 2023

perlpunk Nov 29, 2023

Choose a reason for hiding this comment

josegomezr commented Dec 6, 2023 • edited Loading

okurz commented Dec 7, 2023

josegomezr commented Dec 14, 2023

foursixnine commented Jan 12, 2024 • edited Loading

okurz commented Jan 12, 2024

foursixnine commented Jan 12, 2024

josegomezr commented Jan 14, 2024 • edited Loading

mergify bot commented Jan 14, 2024

okurz commented Jan 15, 2024

josegomezr commented Nov 8, 2023 •

edited

Loading

josegomezr commented Dec 6, 2023 •

edited

Loading

foursixnine commented Jan 12, 2024 •

edited

Loading

josegomezr commented Jan 14, 2024 •

edited

Loading