URL: https://github.com/freeipa/freeipa/pull/5983 Author: stanislavlevin Title: #5983: Azure: Run pycodestyle check in Lint job Action: opened
PR body: """ - previously, fastlint make's target includes both the Pylint task and pycodestyle one. The purpose of this target is a fast checking only for changed Python files. This makes sense for pycodestyle, but limits Pylint due to a context(file) checking. The clients which call the code being linted are not checked at all. In Azure Pylint (for the whole codebase) is run in the Lint task, this makes fastlint extra for Azure. - `Quick code style check` task used distro's Pylint, while `Lint` task PyPI's one. This may cause different results and confuse a user. - `Build` task takes time longer than `Lint` one, so this change doesn't lead to increased CI time. - all Azure tests depend on Build and Lint tasks. Mostly it's no need to run tests due to a probably broken code. Fixes: https://pagure.io/freeipa/issue/8961 """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/5983/head:pr5983 git checkout pr5983
From 536c9884a7f49ca2a41646934a5adf8d46b9e637 Mon Sep 17 00:00:00 2001 From: Stanislav Levin <s...@altlinux.org> Date: Mon, 19 Apr 2021 17:20:47 +0300 Subject: [PATCH] Azure: Run pycodestyle check in Lint job - previously, fastlint make's target includes both the Pylint task and pycodestyle one. The purpose of this target is a fast checking only for changed Python files. This makes sense for pycodestyle, but limits Pylint due to a context(file) checking. The clients which call the code being linted are not checked at all. In Azure Pylint (for the whole codebase) is run in the Lint task, this makes fastlint extra for Azure. - `Quick code style check` task used distro's Pylint, while `Lint` task PyPI's one. This may cause different results and confuse a user. - `Build` task takes time longer than `Lint` one, so this change doesn't lead to increased CI time. - all Azure tests depend on Build and Lint tasks. Mostly it's no need to run tests due to a probably broken code. Fixes: https://pagure.io/freeipa/issue/8961 Signed-off-by: Stanislav Levin <s...@altlinux.org> --- Makefile.am | 35 +++++++++++++++++++++++++----- ipatests/azure/azure-pipelines.yml | 20 ++++++++++------- 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/Makefile.am b/Makefile.am index 321df05a7c4..abeaca7edbe 100644 --- a/Makefile.am +++ b/Makefile.am @@ -216,7 +216,7 @@ endif $(MAKE) $(AM_MAKEFLAGS) acilint apilint polint pylint jslint $(RPMLINT_TARGET) yamllint check @echo "All tests passed." -.PHONY: fastcheck fasttest fastlint +.PHONY: fastcheck fasttest fastlint fastcodestyle fastcheck: @$(MAKE) -j1 $(AM_MAKEFLAGS) fastlint $(RPMLINT_TARGET) yamllint fasttest apilint acilint @@ -229,7 +229,34 @@ fasttest: $(GENERATED_PYTHON_FILES) ipasetup.py --ignore $(abspath $(top_srcdir))/ipatests/test_integration \ --ignore $(abspath $(top_srcdir))/ipatests/test_xmlrpc -fastlint: $(GENERATED_PYTHON_FILES) ipasetup.py acilint apilint +fastcodestyle: $(GENERATED_PYTHON_FILES) ipasetup.py + @echo "Fast code style checking with $(PYTHON) from branch '$(GIT_BRANCH)'" + + @MERGEBASE=$$(git merge-base --fork-point $(GIT_BRANCH)); \ + PYFILES=$$(git diff --name-only --diff-filter=d $${MERGEBASE} \ + | grep -E '\.py$$' ); \ + INFILES=$$(git diff --name-only --diff-filter=d $${MERGEBASE} \ + | grep -E '\.in$$' \ + | xargs -n1 file 2>/dev/null | grep Python \ + | cut -d':' -f1; ); \ + if [ -n "$${PYFILES}" ] && [ -n "$${INFILES}" ]; then \ + FILES="$$( printf $${PYFILES}\\n$${INFILES} )" ; \ + elif [ -n "$${PYFILES}" ]; then \ + FILES="$${PYFILES}" ; \ + else \ + FILES="$${INFILES}" ; \ + fi ; \ + if [ -n "$${FILES}" ]; then \ + echo -e "Fast code style checking for files:\n$${FILES}\n"; \ + echo "pycodestyle"; \ + echo "-----------"; \ + git diff -U0 $${MERGEBASE} | \ + $(PYTHON) -m pycodestyle --diff || exit $$?; \ + else \ + echo "No modified Python files found"; \ + fi + +fastlint: $(GENERATED_PYTHON_FILES) ipasetup.py fastcodestyle acilint apilint if ! WITH_PYLINT @echo "ERROR: pylint not available"; exit 1 endif @@ -251,10 +278,6 @@ endif fi ; \ if [ -n "$${FILES}" ]; then \ echo -e "Fast linting files:\n$${FILES}\n"; \ - echo "pycodestyle"; \ - echo "-----------"; \ - git diff -U0 $${MERGEBASE} | \ - $(PYTHON) -m pycodestyle --diff || exit $$?; \ echo -e "\npylint"; \ echo "------"; \ $(PYTHON) -m pylint --version; \ diff --git a/ipatests/azure/azure-pipelines.yml b/ipatests/azure/azure-pipelines.yml index edf26ad77f8..a920f2852c5 100644 --- a/ipatests/azure/azure-pipelines.yml +++ b/ipatests/azure/azure-pipelines.yml @@ -20,12 +20,6 @@ jobs: steps: - template: templates/${{ variables.PREPARE_BUILD_TEMPLATE }} - template: templates/${{ variables.AUTOCONF_TEMPLATE }} - - script: | - set -e - git update-ref refs/heads/$(System.PullRequest.TargetBranch) origin/$(System.PullRequest.TargetBranch) - make V=0 "GIT_BRANCH=$(System.PullRequest.TargetBranch)" fastlint - displayName: Quick code style check - condition: eq(variables['Build.Reason'], 'PullRequest') - template: templates/${{ variables.BUILD_TEMPLATE }} - template: templates/publish-build.yml parameters: @@ -75,6 +69,12 @@ jobs: echo "Running make target 'lint'" make V=0 lint displayName: Lint sources + - script: | + set -e + git update-ref refs/heads/$(System.PullRequest.TargetBranch) origin/$(System.PullRequest.TargetBranch) + make V=0 "GIT_BRANCH=$(System.PullRequest.TargetBranch)" fastcodestyle + displayName: Quick code style check + condition: eq(variables['Build.Reason'], 'PullRequest') - job: Docs pool: @@ -144,7 +144,9 @@ jobs: - job: BASE_XMLRPC pool: vmImage: $(VM_IMAGE) - dependsOn: Build + dependsOn: + - Build + - Lint variables: IPA_IMAGE_ARTIFACT: $[ dependencies.Build.outputs['artifacts_image.image'] ] IPA_PACKAGES_ARTIFACT: $[ dependencies.Build.outputs['artifacts_packages.packages'] ] @@ -158,7 +160,9 @@ jobs: - job: GATING pool: vmImage: $(VM_IMAGE) - dependsOn: Build + dependsOn: + - Build + - Lint variables: IPA_IMAGE_ARTIFACT: $[ dependencies.Build.outputs['artifacts_image.image'] ] IPA_PACKAGES_ARTIFACT: $[ dependencies.Build.outputs['artifacts_packages.packages'] ]
_______________________________________________ FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/freeipa-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
