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

Unify perl linting rules #5348

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

josegomezr
Copy link
Contributor

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

@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.32%. Comparing base (f8f5bc4) to head (5b1d054).
Report is 1366 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5348   +/-   ##
=======================================
  Coverage   98.32%   98.32%           
=======================================
  Files         389      389           
  Lines       37319    37319           
=======================================
  Hits        36693    36693           
  Misses        626      626           

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

@josegomezr josegomezr force-pushed the experiment_unify_perl_lint_rules branch from 8811a7a to 5b1d054 Compare October 26, 2023 07:45
-fbl # don't change blank lines
-fnl # don't remove new lines
Copy link
Member

Choose a reason for hiding this comment

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

I agree to include this line but I do not want to loosen the line-length rule. We must find a way to support project specific rule additions and exceptions.

Copy link
Member

Choose a reason for hiding this comment

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

As I remember the conversation from the other PR there's no easy way to add exceptions per project. Could 120 be used instead?

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 covered that we can not easily reduce os-autoinst-distri-opensuse from 160 down to 120

Copy link
Member

Choose a reason for hiding this comment

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

Okay. I guess I'm fine with 160 then if it's the only deviation.

Copy link
Contributor

@perlpunk perlpunk Nov 6, 2023

Choose a reason for hiding this comment

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

I think we can live with a .perltidyrc per project. If the common project has a config, existing and new projects can use this by symlinking, but they don't have to, so openQA would keep its own config, if the common project uses 160, or if the common project uses 120, distri-opensuse would use its own.
That's at least what @josegomezr agreed on when we chatted today

@josegomezr josegomezr changed the title experiment: Unify perl linting rules Unify perl linting rules Nov 6, 2023
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.

Works for me.

Copy link
Contributor

mergify bot commented Apr 12, 2024

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

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.

5 participants