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.

Sign up for GitHub

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

Jump to bottom

Use new perltidy wrapper tools/tidyall #5838

Merged
merged 2 commits into from
Aug 9, 2024
Merged

Use new perltidy wrapper tools/tidyall #5838

merged 2 commits into from
Aug 9, 2024

Conversation

perlpunk
Copy link
Contributor

@perlpunk perlpunk commented Aug 8, 2024 •
edited
Loading

@perlpunk
git subrepo pull (merge) external/os-autoinst-common
6c00285 
subrepo:
  subdir:   "external/os-autoinst-common"
  merged:   "9a505a76f"
upstream:
  origin:   "[email protected]:os-autoinst/os-autoinst-common.git"
  branch:   "master"
  commit:   "9a505a76f"
git-subrepo:
  version:  "0.4.6"
  origin:   "[email protected]:ingydotnet/git-subrepo"
  commit:   "110b9eb"
@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 8, 2024 •
edited by perlpunk
Loading

Great PR! Please pay attention to the following items before merging:

Files matching docs/*.asciidoc:

  • Consider generating documentation locally to verify it is rendered correctly using tools/generate-docs

Files matching external/**:

  • Consider doing this change in the upstream repository directly, see external/os-autoinst-common/README.md

This is an automatically generated QA checklist based on modified files.

@perlpunk perlpunk changed the title Tidyall Use new perltidy wrapper tools/tidyall Aug 8, 2024
@perlpunk perlpunk force-pushed the tidyall branch 2 times, most recently from 90aa815 to 1f5a116 Compare August 8, 2024 10:33
@perlpunk
Copy link
Contributor Author

perlpunk commented Aug 8, 2024

This is now failing because it tries to tidy everything under script/, also bash scripts.
In the previous tools/tidy wrapper we were using a helper tools/perlfiles to find all relevant files.
In the .tidyallrc we can't use such a helper script.
So it would be good to add another option to the tidyall wrapper that allows us to use that helper.

@perlpunk
Copy link
Contributor Author

perlpunk commented Aug 8, 2024 •
edited
Loading

I now added shebang = perl to .tidyallrc
https://metacpan.org/dist/Code-TidyAll/view/bin/tidyall#shebang

hm. Apparently it now doesn't tidy .pm files anymore...

@perlpunk perlpunk marked this pull request as draft August 8, 2024 15:23
@perlpunk
Copy link
Contributor Author

perlpunk commented Aug 8, 2024

I think I now found a configuration that works for all.
https://metacpan.org/dist/Code-TidyAll/view/bin/tidyall#[class]-or-[class-description]
We can have more than one section. In one we specify the files with a file extension, in the other we specify the scripts and use the shebang option.

@perlpunk perlpunk marked this pull request as ready for review August 8, 2024 15:52
b10n1k
b10n1k reviewed Aug 9, 2024
View reviewed changes
.tidyallrc Show resolved Hide resolved
@perlpunk
Copy link
Contributor Author

perlpunk commented Aug 9, 2024

We don't get codecov patch info, but that's not relevant here, so merging

@perlpunk perlpunk merged commit 9afc3ef into os-autoinst:master Aug 9, 2024
38 checks passed
@perlpunk perlpunk deleted the tidyall branch August 9, 2024 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@b10n1k b10n1k b10n1k left review comments

@baierjan baierjan baierjan approved these changes

@okurz okurz okurz approved these changes

@kalikiana kalikiana kalikiana approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@perlpunk @b10n1k @kalikiana @okurz @baierjan