#7 Adding CI and pre-commit hooks
Merged 4 years ago by nphilipp. Opened 4 years ago by asaleh.
fedora-infra/ asaleh/rpmautospec adding_ci_and_precommit_hooks  into  master

@@ -0,0 +1,16 @@ 

+ repos:

+ -   repo: https://github.com/pre-commit/pre-commit-hooks

+     rev: v2.4.0

+     hooks:

+     -   id: check-json

+     -   id: check-added-large-files

+     -   id: check-yaml

+     -   id: debug-statements

+ -   repo: https://gitlab.com/pycqa/flake8

+     rev: 3.7.1

+     hooks:

+     -   id: flake8

+ -   repo: https://github.com/psf/black

+     rev: 19.10b0

+     hooks:

+     -   id: black

file modified
+10 -1
@@ -1,4 +1,13 @@ 

+ - job:

+     name: pre-commit

+     run: ci/pre-commit.yaml

+     nodeset: fedora-31-vm

+ - job:

+     name: pytest

+     run: ci/pytest.yaml

+     nodeset: fedora-31-vm

  - project:

      check:

        jobs:

-         - noop

+         - pre-commit

+         - pytest

file added
+13
@@ -0,0 +1,13 @@ 

+  - hosts: all

+    tasks:

+     - name: List project directory on the test system

+       command: ls -al {{ansible_user_dir}}/{{zuul.project.src_dir}}

+     - name: install pip

+       become: yes

+       package:

+         name: python3-pip

+         state: present

+     - name: install pre-comit

+       command: pip install pre-commit --user

+     - name: run pre-comit

+       command: chdir={{ansible_user_dir}}/{{zuul.project.src_dir}} pre-commit run --all

file added
+11
@@ -0,0 +1,11 @@ 

+  - hosts: all

+    tasks:

+     - name: List project directory on the test system

+       command: ls -al {{ansible_user_dir}}/{{zuul.project.src_dir}}

+     - name: install deps

+       become: yes

+       package:

+         name: ['python3-pytest', 'python3-rpm', 'python3-koji', 'python3-pygit2']

+         state: present

+     - name: run pytest

+       command: chdir={{ansible_user_dir}}/{{zuul.project.src_dir}} python -m pytest

@@ -38,7 +38,7 @@ 

                      os.path.pardir,

                      "test-data",

                      "repodata",

-                     "dummy-test-package-gloster_git.tar.gz"

+                     "dummy-test-package-gloster_git.tar.gz",

                  )

              ) as tar:

                  tar.extractall(path=workdir)

Added pre-commit hooks with the help of pre-commit.com
Runs flake8, mypy and black, checks for leftover debug statements and
additionally validates json and yaml.

CI installs all the dependencies with packages (because python3-rpm)
and runs pytest and pre-commit hooks.

Zuul encountered a syntax error while parsing its configuration in the
repo Fedora-Infra/rpmautospec on branch master. The error was:

extra keys not allowed @ data['check']['jobs'][0]['job']['name']

The error appears in the following project stanza:

project:
check:
jobs:
- job:
name: pre-commit
description: Running pre-commit
run: ci/pre-commit.yaml
- job:
name: pytest
description: Running pytest
run: ci/pytest.yaml

in "Fedora-Infra/rpmautospec/.zuul.yaml@master", line 1, column 3

rebased onto 43c567574ac64de9dff8d321df7ba4872c2f7dab

4 years ago

Zuul encountered a syntax error while parsing its configuration in the
repo Fedora-Infra/rpmautospec on branch master. The error was:

extra keys not allowed @ data['check']['jobs'][0]['job']['name']

The error appears in the following project stanza:

project:
check:
jobs:
- job:
name: pre-commit
description: Running pre-commit
run: ci/pre-commit.yaml
- job:
name: pytest
description: Running pytest
run: ci/pytest.yaml

in "Fedora-Infra/rpmautospec/.zuul.yaml@master", line 1, column 3

rebased onto 080dffc48ac94f03cfe368dfa0c30ee4e1b90b82

4 years ago

Zuul encountered a syntax error while parsing its configuration in the
repo Fedora-Infra/rpmautospec on branch master. The error was:

extra keys not allowed @ data['check']['jobs'][0]['job']['name']

The error appears in the following project stanza:

project:
check:
jobs:
- job:
name: pre-commit
description: Running pre-commit
run: ci/pre-commit.yaml
- job:
name: pytest
description: Running pytest
run: ci/pytest.yaml

in "Fedora-Infra/rpmautospec/.zuul.yaml@master", line 1, column 3

rebased onto 9da085de1d6ce357b92e697a227807c84636e4e0

4 years ago

Zuul encountered a syntax error while parsing its configuration in the
repo Fedora-Infra/rpmautospec on branch master. The error was:

extra keys not allowed @ data['check']['jobs'][0]['job']['name']

The error appears in the following project stanza:

project:
check:
jobs:
- job:
name: pre-commit
description: Running pre-commit
run: ci/pre-commit.yaml
- job:
name: pytest
description: Running pytest
run: ci/pytest.yaml

in "Fedora-Infra/rpmautospec/.zuul.yaml@master", line 1, column 3

rebased onto 7c9bc1cdca73f51354c8627faba2741fec6073c6

4 years ago

Build failed.

jobs must be defined in there own statements as it is done here: https://pagure.io/fedora-qa/os-autoinst-distri-fedora/blob/master/f/.zuul.yaml

jobs must be defined in there own statements as it is done here: https://pagure.io/fedora-qa/os-autoinst-distri-fedora/blob/master/f/.zuul.yaml

that's good now :)

rebased onto 6fdef7a8086836b047001ae6223117095ddec47e

4 years ago

Build failed.

rebased onto 705289a1c69239e6a76c063ff8310ead1b518574

4 years ago

Build failed.

rebased onto 476c991ee6e67cbdcfc12a6b9fc41f2a210a9ca2

4 years ago

Build succeeded.

Metadata Update from @nphilipp:
- Request assigned

4 years ago

Any reason for these blank lines?

Something tells me you don't use an Officially Approvedâ„¢ text editor. :joy:

Why is this file removed?

I wasn't aware that this is possible, interesting. But it's a little over the top, especially if we don't add typing hints to the functions in this module. I'd rather prefix the variable with an underscore to tell tools like mypy that it isn't part of external API.

This doesn't conform to Python naming conventions, see here in PEP8, i.e. it should be compare_evr instead.

Why should we introduce a separate named function compare_evr for this anyway, rather than use a throwaway lambda? It's literally only needed to produce the keying function rpmvercmp_key.

Uhm, why? I don't know of a convention to prefer the type aliases from typing to the type itself. I know re.Pattern and re.Match were hidden in older Python versions (which is why I believe typing.Pattern and typing.Match exist), but this isn't the case anymore.

Not if lpkgrel is not None:?

What's the reason for using a range rather than a tuple?

I'd also like to have the changes in CI configuration split from the changes to e.g. Python sources.

BTW, what's the difference between .pre-commit-config.yaml and ci/pre-commit.yaml?

So, practically all of the changes are due to mypy, some of the reasonable ones are the parts where it flagged possible None (and yes, it should have been if lpkgrel is not None: :) ), others are more of a concession to the type-checker (i.e. both tuple and range are iterable, but it couldn't reconcile those, it couldn't infer the type of the lambda in rpmvercmp_key, but it is fine with additional function defined, neeging to import the Match and Pattern typings are known issue (mypy for some reason doesn't see the export from re?) and it didn't like the rpmautospec.py having same name as the rpmautospec folder.

Because you mention that I probably should have separated the changes to files into a separate commit, I think what I will do, is remove mypy checks and changes from this PR, and create second one where we can discuss if it is worth it :-) So far I am on the fence (i.e. ~half of the changes I would consider useful, i.e. none-checks and checking type signatures)

Wrt .pre-commit-config.yaml - that is the actuall hook configuration, ci/pre-commit.yaml just triggers it in CI.

rebased onto 44fd8da9d6672d31d60694346b56a275804b1041

4 years ago

Build failed.

1 new commit added

  • Fixing line ends
4 years ago

Build failed.

2 new commits added

  • Removing the end-of-line-fixer
  • Added pre-commit hooks with the help of pre-commit.com
4 years ago

Build failed.

1 new commit added

  • Forgot to run black on everything.
4 years ago

Build succeeded.

rebased onto 7e7ddde0b9ae7557b15e1f2937a08b85940ce759

4 years ago

Build succeeded.

rebased onto 36ceb14903073a64217691473b70edf7c8daf80e

4 years ago

Build succeeded.

Pull-Request has been merged by nphilipp

4 years ago