[Merge] ~andersson123/meta-release:pre-commit-config-amendment into meta-release:main
Paride Legovini
mp+463904 at code.launchpad.net
Tue Apr 9 11:56:11 UTC 2024
Review: Needs Fixing
Diff comments:
> diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
> index 726c29b..e44e684 100644
> --- a/.pre-commit-config.yaml
> +++ b/.pre-commit-config.yaml
> @@ -8,22 +8,16 @@ repos:
> - id: url-checker
> name: url-checker
> entry: ./url-checker.sh
> - args:
> - - meta-release
> - - meta-release-development
> - - meta-release-lts
> - - meta-release-lts-development
> - - meta-release-lts-proposed
> - - meta-release-proposed
> - - meta-release-unit-testing
> - language: system
> - pass_filenames: false
> + language: script
> + types: [file]
> + verbose: true
I would not go with verbose: it is nice that pre-commit doesn't get in the way by printing extra stuff when everything is fine.
> + files: meta-release*
that * is useless, it basically means "match any repetition of the last letter":
meta-release
meta-releaseeeee
meta-releaseeeeeeeeeeeeee
...
This still appears to work because re.search() matches substrings.
Suggestion: make it a proper regexp like you did for the .json check.
> always_run: true
why always_run?
> - id: json-checker
> name: json-checker
> entry: ./json-checker.sh
> - args:
> - - raspi/os_list_imagingutility_ubuntu.json
> - language: system
> - pass_filenames: false
> + language: script
> + types: [json]
> + verbose: true
> + files: ^raspi\/.+.json$
I don't think the / needs escaping, OTOH the . in .json does.
> always_run: true
same
--
https://code.launchpad.net/~andersson123/meta-release/+git/main/+merge/463904
Your team Ubuntu Core Development Team is subscribed to branch meta-release:main.
More information about the Ubuntu-reviews
mailing list