canonical-ubuntu-qa team mailing list archive

[Paride Legovini <mp+445630@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> diff --git a/.launchpad.yaml b/.launchpad.yaml
> index a744b71..bec3a7d 100755
> --- a/.launchpad.yaml
> +++ b/.launchpad.yaml
> @@ -1,16 +1,39 @@
>  pipeline:
>    - pre_commit
>    - build_charms
> -  - lint_test
>  
>  jobs:
>    pre_commit:
> -    series: jammy
> +    series: focal
>      architectures: amd64
> +    package-repositories:
> +      - type: apt
> +        formats: [deb]
> +        suites: [focal]
> +        ppa: paride/pre-commit-backports
> +    snaps:
> +      - name: yq
>      packages:
>        - git
>        - pre-commit
> -    run: pre-commit run --all-files --show-diff-on-failure
> +      - pylint
> +      # These are not directly declared in layer.yaml.
> +      # Maybe they are brought in by some included layer?
> +      - python3-systemd
> +      - python3-apt
> +    run-before: |
> +      # Install dependencies as declared in the layer files
> +      DEBIAN_FRONTEND=noninteractive \
> +        apt-get install -qy \
> +        $(cat charms/focal/autopkgtest-cloud-worker/layer.yaml | \
> +          yq -r '.options.basic.packages[]') \
> +        $(cat charms/focal/autopkgtest-cloud-worker/layer.yaml | \
> +          yq -r '.options.apt.packages[]') \
> +        $(cat charms/focal/autopkgtest-web/layer.yaml | \
> +          yq -r '.options.apt.packages[]') \

One time for autopkgtest-cloud-worker the other for autopkgtest-web, but you're right I changed the order. Little cosmetic thing but I'll amend it.

> +        $(cat charms/focal/autopkgtest-web/layer.yaml | \
> +          yq -r '.options.basic.packages[]')
> +    run: pre-commit run --hook-stage manual --all-files --show-diff-on-failure
>    build_charms:
>      series: focal
>      architectures: amd64
> @@ -18,8 +41,3 @@ jobs:
>        - name: charmcraft
>          classic: true
>      run: ./ci/build_charms
> -  lint_test:
> -    series: focal
> -    architectures: amd64
> -    packages: [pylint, python3, shellcheck, yamllint]
> -    run: ./ci/lint_test

Let's have a separate MP, I'll drop these changes from this one and have a separate one with this diff and also removing the script.

> diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
> index 0735120..d3bc39f 100644
> --- a/.pre-commit-config.yaml
> +++ b/.pre-commit-config.yaml
> @@ -22,3 +22,28 @@ repos:
>      rev: 5.12.0
>      hooks:
>        - id: isort
> +  - repo: local
> +    hooks:
> +      # Run pylint with --disable import-error.
> +      # This is meant to run on developers' machines where not all the
> +      # Python modules may be installed.
> +      - id: pylint
> +        name: pylint
> +        stages:
> +          [commit-msg, post-checkout, post-commit, post-merge, post-rewrite,
> +           pre-commit, pre-merge-commit, pre-push, pre-rebase,
> +           prepare-commit-msg]
> +        entry: pylint
> +        args:
> +          - "--disable=import-error"
> +        language: system
> +        types: [python]
> +      # Run pylint without disabling import-error.
> +      # This is meant to run in CI (pre-commit run --hook-stage manual).
> +      # The CI environment is expected to have all the required dependencies.
> +      - id: pylint
> +        name: pylint (with import errors)
> +        stages: [manual]
> +        entry: pylint
> +        language: system
> +        types: [python]

In CI it will run only *without* the disable. The one without the disable has

  stages: [manual]

so it will only run in CI where we run (check .launchpad.yaml)

  pre-commit run --hook-stage manual

The one *with* disable has all the stages but the manual one, so it won't run in CI. (There is no way to exclude a stage, so I had to explicitly list all of them without 'manual'.)



-- 
https://code.launchpad.net/~paride/autopkgtest-cloud/+git/autopkgtest-cloud/+merge/445630
Your team Canonical's Ubuntu QA is requested to review the proposed merge of ~paride/autopkgtest-cloud:pre-commit-pylint into autopkgtest-cloud:master.