[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