From fad50c3586c117fcafa12c832eb8d46a3a986ea6 Mon Sep 17 00:00:00 2001 From: Oliver Koenig Date: Fri, 8 Nov 2024 16:00:55 +0100 Subject: [PATCH 1/5] ci: Improve PyLint handling Signed-off-by: Oliver Koenig --- .github/workflows/code-formatting.yml | 127 ++++++++++++++++++++++++-- 1 file changed, 121 insertions(+), 6 deletions(-) diff --git a/.github/workflows/code-formatting.yml b/.github/workflows/code-formatting.yml index 5484da5fdcd9..3e6afe9b7501 100644 --- a/.github/workflows/code-formatting.yml +++ b/.github/workflows/code-formatting.yml @@ -10,7 +10,7 @@ name: Isort and Black Formatting; PyLint Docs check # For details see https://github.com/orgs/community/discussions/25702 on: - pull_request_target: + pull_request: #pull_request_target: paths: - '**.py' types: [ opened, synchronize, reopened, labeled, unlabeled ] @@ -20,7 +20,7 @@ defaults: shell: bash -x -e -u -o pipefail {0} jobs: - reformat_with_isort_and_black_and_pylint: + reformat_with_isort_and_black: runs-on: ubuntu-latest permissions: # write permissions required to commit changes @@ -72,16 +72,131 @@ jobs: message: Apply isort and black reformatting commit: --signoff + check_pylint: + name: "check_pylint (strict-mode: ${{ matrix.strict-mode }})" + runs-on: ubuntu-latest + permissions: + contents: write + pull-requests: write + env: + THRESHOLD: 1730937600 # On this date (2024/11/07) we decided to add Pylint. It shall only run in strict mode for files added past this date. For files prior to this date, we will only add a PR comment with PyLint's stdout. + strategy: + matrix: + strict-mode: ["true", "false"] + steps: + - name: Checkout branch + uses: actions/checkout@v4 + with: + # setup repository and ref for PRs, see + # https://github.com/EndBug/add-and-commit?tab=readme-ov-file#working-with-prs + repository: ${{ github.event.pull_request.head.repo.full_name }} + ref: ${{ github.event.pull_request.head.ref }} + fetch-depth: 0 + + # https://github.com/tj-actions/changed-files + - name: Get changed files + id: changed-files + uses: tj-actions/changed-files@v44 + with: + files: | + **.py + + - name: Setup Python env + uses: actions/setup-python@v5 + with: + python-version: "3.10" + - name: pylint if: ${{ steps.changed-files.outputs.any_changed == 'true' && !contains( github.event.pull_request.labels.*.name, 'skip-docs') }} + id: pylint env: # only *.py files included + STRICT_MODE: ${{ matrix.strict-mode }} CHANGED_FILES: "${{ steps.changed-files.outputs.all_changed_files }}" run: | pip install pylint - ADDITIONAL_PYLINT_ARGS=() - echo ${{ github.event.pull_request.labels.*.name }} + FILTERED=() + for file in $CHANGED_FILES; do + DATE=$(git log --format=%ad --date=unix $file | tail -1) + + if [[ "$STRICT_MODE" == "true" ]]; then + if [[ "$DATE" -gt "$THRESHOLD" ]]; then + FILTERED+=("$file") + fi + else + if [[ "$DATE" -le "$THRESHOLD" ]]; then + FILTERED+=("$file") + fi + fi + done + + if [ ${#FILTERED[@]} -eq 0 ]; then + echo "No files to check." + exit 0 + fi + + echo "Will run on these files: + ${FILTERED[@]}" + + set +e + LOG=$(pylint ${FILTERED[@]}) + EXIT_CODE=$? + set -e + + echo "$LOG" + echo "OUTPUT<> $GITHUB_ENV + echo "$LOG" >> $GITHUB_ENV + echo "EOF" >> $GITHUB_ENV + echo "log=$LOG" + + if [[ "${{ matrix.strict-mode }}" == "true" ]]; then + HEADER="🚨 The following files must be fixed before merge!" + else + HEADER="🙏 The following files have warnings. In case you are familiar with these, please try helping us to improve the code base." + fi + echo "header=$HEADER" | tee -a "$GITHUB_OUTPUT" + + exit $([[ "$EXIT_CODE" -ne 0 && "$STRICT_MODE" == "true" ]] && echo $EXIT_CODE || echo 0) + + - name: Find Comment + if: ${{ always() && env.OUTPUT != '' }} + uses: peter-evans/find-comment@v3 + id: fc + with: + issue-number: ${{ github.event.number }} + body-includes: + + - name: Delete comment + if: ${{ always() && env.OUTPUT != '' && steps.fc.outputs.comment-id != '' }} + env: + GH_TOKEN: ${{ secrets.github_token }} + REPOSITORY: ${{ github.repository }} + COMMENT_ID: ${{ steps.fc.outputs.comment-id }} + run: | + curl -L \ + -X DELETE \ + -H "Accept: application/vnd.github+json" \ + -H "Authorization: Bearer $GH_TOKEN" \ + -H "X-GitHub-Api-Version: 2022-11-28" \ + https://api.github.com/repos/$REPOSITORY/issues/comments/$COMMENT_ID + + - name: Add PR comment for PyLint + if: ${{ always() && env.OUTPUT != '' }} + uses: peter-evans/create-or-update-comment@v4 + with: + issue-number: ${{ github.event.number }} + body: | + + + ${{ steps.pylint.outputs.header }} + + --- + + Your code was analyzed with PyLint. The following annotations have been identified: + + ``` + ${{ env.OUTPUT }} + ``` - pylint ${ADDITIONAL_PYLINT_ARGS[@]} $CHANGED_FILES || \ - { echo "Pylint failed. In case of long strings, format docstrings and other strings manually."; exit 1; } + Please help us to raise the code-quality of NeMo! From 65401326f08e7dee63e6f501029bdb4a0cc3dc8f Mon Sep 17 00:00:00 2001 From: Oliver Koenig Date: Fri, 8 Nov 2024 16:01:08 +0100 Subject: [PATCH 2/5] finalize Signed-off-by: Oliver Koenig --- .github/workflows/code-formatting.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/code-formatting.yml b/.github/workflows/code-formatting.yml index 3e6afe9b7501..340e7a6996fa 100644 --- a/.github/workflows/code-formatting.yml +++ b/.github/workflows/code-formatting.yml @@ -10,7 +10,7 @@ name: Isort and Black Formatting; PyLint Docs check # For details see https://github.com/orgs/community/discussions/25702 on: - pull_request: #pull_request_target: + pull_request_target: paths: - '**.py' types: [ opened, synchronize, reopened, labeled, unlabeled ] From 295d6a50cead46cb76009c0762ab22de6aa12ddd Mon Sep 17 00:00:00 2001 From: Oliver Koenig Date: Fri, 8 Nov 2024 16:04:20 +0100 Subject: [PATCH 3/5] bot Signed-off-by: Oliver Koenig --- .github/workflows/code-formatting.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/code-formatting.yml b/.github/workflows/code-formatting.yml index 340e7a6996fa..dc9a3f9e7631 100644 --- a/.github/workflows/code-formatting.yml +++ b/.github/workflows/code-formatting.yml @@ -189,7 +189,7 @@ jobs: body: | - ${{ steps.pylint.outputs.header }} + beep boop 🤖: ${{ steps.pylint.outputs.header }} --- From f6d2c2d794f220fc94fed6a2127a907107f3cc69 Mon Sep 17 00:00:00 2001 From: Oliver Koenig Date: Fri, 8 Nov 2024 16:05:04 +0100 Subject: [PATCH 4/5] fix Signed-off-by: Oliver Koenig --- .github/workflows/code-formatting.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/code-formatting.yml b/.github/workflows/code-formatting.yml index dc9a3f9e7631..59342158433b 100644 --- a/.github/workflows/code-formatting.yml +++ b/.github/workflows/code-formatting.yml @@ -199,4 +199,6 @@ jobs: ${{ env.OUTPUT }} ``` + --- + Please help us to raise the code-quality of NeMo! From 309a91806b4f2f89bfb7452f821697b6c3e2fc83 Mon Sep 17 00:00:00 2001 From: Oliver Koenig Date: Fri, 8 Nov 2024 16:05:43 +0100 Subject: [PATCH 5/5] fix Signed-off-by: Oliver Koenig --- .github/workflows/code-formatting.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/code-formatting.yml b/.github/workflows/code-formatting.yml index 59342158433b..b08e9676aabd 100644 --- a/.github/workflows/code-formatting.yml +++ b/.github/workflows/code-formatting.yml @@ -200,5 +200,5 @@ jobs: ``` --- - - Please help us to raise the code-quality of NeMo! + + Thank you for improving NeMo's documentation!