diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 9c41a23d011..7eedc9e7bac 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -21,7 +21,8 @@ env:
COLUMNS: 160
permissions:
- contents: read
+ contents: read # for dorny/paths-filter to fetch a list of changed files
+ pull-requests: read # for dorny/paths-filter to read pull requests
concurrency:
# Concurrency is defined in a way that concurrent builds against branches do
@@ -35,9 +36,6 @@ jobs:
prepare-ci:
name: Prepare CI
runs-on: ubuntu-latest
- permissions:
- contents: read # for dorny/paths-filter to fetch a list of changed files
- pull-requests: read # for dorny/paths-filter to read pull requests
outputs:
jobs: ${{ steps.process-changed-files.outputs.jobs }}
changed-files: ${{ steps.process-changed-files.outputs.changed-files }}
@@ -200,20 +198,6 @@ jobs:
with:
changed-files: ${{ needs.prepare-ci.outputs.changed-files }}
- pr-checks:
- name: PR Checks
- if: ${{ fromJSON(needs.prepare-ci.outputs.jobs)['github-hosted-runners'] && github.event_name == 'pull_request' }}
- permissions:
- contents: read
- pull-requests: write
- uses: ./.github/workflows/pr-checks-action.yml
- needs:
- - prepare-ci
- with:
- changed-files: ${{ needs.prepare-ci.outputs.changed-files }}
- secrets:
- github-token: ${{ secrets.GITHUB_TOKEN }}
-
windows-2016:
name: Windows 2016
if: ${{ fromJSON(needs.prepare-ci.outputs.jobs)['self-hosted-runners'] }}
@@ -479,7 +463,6 @@ jobs:
- docs
- lint
- twine-check
- - pr-checks
- almalinux-8
- almalinux-9
- amazonlinux-2
diff --git a/.github/workflows/pr-checks-action.yml b/.github/workflows/pr-checks-action.yml
deleted file mode 100644
index 6e40a83fa46..00000000000
--- a/.github/workflows/pr-checks-action.yml
+++ /dev/null
@@ -1,62 +0,0 @@
-name: PR Checks
-
-on:
- workflow_call:
- inputs:
- changed-files:
- required: true
- type: string
- description: JSON string containing information about changed files
- secrets:
- github-token:
- description: The GitHub token to use to comment on PRs
- required: true
-
-permissions:
- contents: read
- pull-requests: write
-
-jobs:
-
- Check-Changed-Files-Docstrings:
- name: Check Docstrings For Changed Files On PR
- runs-on: ubuntu-latest
- if: ${{ github.event_name == 'pull_request' && fromJSON(inputs.changed-files)['salt'] }}
-
- steps:
- - uses: actions/checkout@v3
- - uses: actions/setup-python@v4
- with:
- python-version: '3.9'
-
- - name: Install Dependencies
- env:
- PIP_EXTRA_INDEX_URL: https://pypi-proxy.saltstack.net/root/local/+simple/
- run: |
- python -m pip install --upgrade pip
- pip install pre-commit pygithub
-
- - name: Install Pre-Commit Hooks
- run: |
- pre-commit install --install-hooks
-
- - name: Check Docstrings
- id: check-known-missing-docstrings
- continue-on-error: true
- shell: bash
- run: |
- set -o pipefail
- pre-commit run --hook-stage manual check-known-missing-docstrings \
- --show-diff-on-failure --color=never \
- --files ${{ join(fromJSON(inputs.changed-files)['salt_files'], ' ') }} | tee output.txt
-
- - name: Comment on PR
- # Comment on PRs if pre-commit triggered a failure
- if: steps.check-known-missing-docstrings.outcome == 'failure'
- env:
- GITHUB_TOKEN: ${{ secrets.github-token }}
- run: |
- python .github/workflows/scripts/pr-docstring-comments.py \
- --org ${{ github.repository_owner }} \
- --repo ${{ github.event.repository.name }} \
- --issue ${{ github.event.number }} output.txt
diff --git a/.github/workflows/pre-commit-action.yml b/.github/workflows/pre-commit-action.yml
index 000587e2b24..242ce6b1ca2 100644
--- a/.github/workflows/pre-commit-action.yml
+++ b/.github/workflows/pre-commit-action.yml
@@ -47,6 +47,7 @@ jobs:
if: github.event_name == 'pull_request' && fromJSON(inputs.changed-files)['repo']
env:
SKIP: lint-salt,lint-tests
+ GH_ACTIONS_ANNOTATE: "1"
run: |
pre-commit run --show-diff-on-failure --color=always --files ${{ join(fromJSON(inputs.changed-files)['repo_files'], ' ') }}
diff --git a/.github/workflows/scripts/pr-docstring-comments.py b/.github/workflows/scripts/pr-docstring-comments.py
deleted file mode 100644
index 6fdbc7dd113..00000000000
--- a/.github/workflows/scripts/pr-docstring-comments.py
+++ /dev/null
@@ -1,124 +0,0 @@
-import argparse
-import os
-import pathlib
-import sys
-
-import github
-from github.GithubException import GithubException
-
-COMMENT_HEADER = "### Hi! I'm your friendly PR bot!"
-COMMENT_TEMPLATE = """\
-{comment_header}
-
-You might be wondering what I'm doing commenting here on your PR.
-
-**Yes, as a matter of fact, I am...**
-
-I'm just here to help us improve the documentation. I can't respond to
-questions or anything, but what I *can* do, I do well!
-
-**Okay... so what do you do?**
-
-I detect modules that are missing docstrings or "CLI Example" on existing docstrings!
-When I was created we had a *lot* of these. The documentation for these
-modules need some love and attention to make Salt better for our users.
-
-**So what does that have to do with my PR?**
-
-I noticed that in this PR there are some files changed that have some of these
-issues. So I'm leaving this comment to let you know your options.
-
-**Okay, what are they?**
-
-Well, my favorite, is that since you were making changes here I'm hoping that
-you would be the most familiar with this module and be able to add some other
-examples or fix any of the reported issues.
-
-**If I can, then what?**
-
-Well, you can either add them to this PR or add them to another PR. Either way is fine!
-
-**Well... what if I can't, or don't want to?**
-
-That's also fine! We appreciate *all* contributions to the Salt Project. If you
-can't add those other examples, either because you're too busy, or unfamiliar,
-or you just aren't interested, we still appreciate the contributions that
-you've made already.
-
-Whatever approach you decide to take, just drop a comment here letting us know!
-
-
-Detected Issues (click me)
-{issues_output}
-
-
----
-
-Thanks again!
-"""
-
-
-def get_previous_comments(pr):
- for comment in pr.get_issue_comments():
- if comment.user.login != "github-actions[bot]":
- # Not a comment made by this bot
- continue
- if not comment.body.startswith(COMMENT_HEADER):
- # This comment does not start with our header
- continue
- yield comment
-
-
-def comment_on_pr(options, issues_output):
- gh = github.Github(os.environ["GITHUB_TOKEN"])
- org = gh.get_organization(options.org)
- print(f"Loaded Organization: {org.login}", file=sys.stderr, flush=True)
- repo = org.get_repo(options.repo)
- print(f"Loaded Repository: {repo.full_name}", file=sys.stderr, flush=True)
- pr = repo.get_pull(options.issue)
- print(f"Loaded PR: {pr}", file=sys.stderr, flush=True)
- comment = pr.create_issue_comment(
- COMMENT_TEMPLATE.format(
- comment_header=COMMENT_HEADER, issues_output=issues_output
- )
- )
- new_comment_content = COMMENT_TEMPLATE.format(
- comment_header=COMMENT_HEADER, issues_output=issues_output
- )
- for comment in get_previous_comments(pr):
- if comment.body.strip() != new_comment_content.strip():
- # The content has changed.
- print(f"Deleting previous comment {comment}")
- comment.delete()
-
- comment = pr.create_issue_comment(new_comment_content)
- print(f"Created Comment: {comment}")
-
-
-def main():
- parser = argparse.ArgumentParser()
- parser.add_argument("--org", required=True, help="The Github Organization")
- parser.add_argument("--repo", required=True, help="The Organization Repository")
- parser.add_argument("--issue", required=True, type=int, help="The issue number")
- parser.add_argument(
- "issues_output_path", metavar="ISSUES_OUTPUT_PATH", type=pathlib.Path
- )
-
- if not os.environ.get("GITHUB_TOKEN"):
- parser.exit(status=1, message="GITHUB_TOKEN environment variable not set")
-
- options = parser.parse_args()
- if not options.issues_output_path.is_file():
- parser.exit(1, message=f"The path {options.issues_output_path} is not a file")
- issues_output = options.issues_output_path.read_text().strip()
- if not issues_output:
- parser.exit(1, message=f"The file {options.issues_output_path} is empty")
- try:
- comment_on_pr(options, issues_output)
- parser.exit(0)
- except GithubException as exc:
- parser.exit(1, message=str(exc))
-
-
-if __name__ == "__main__":
- main()
diff --git a/tasks/docstrings.py b/tasks/docstrings.py
index b8078c6dfe2..3aed5c7fa87 100644
--- a/tasks/docstrings.py
+++ b/tasks/docstrings.py
@@ -4,10 +4,14 @@
Docstrings related tasks
"""
+# pylint: disable=resource-leakage
import ast
+import os
import pathlib
import re
+import sys
+from typing import TYPE_CHECKING
from invoke import task # pylint: disable=3rd-party-module-not-gated
@@ -782,6 +786,72 @@ MISSING_EXAMPLES = {
],
}
+SUMMARY = """\
+### Hi! I'm your friendly PR bot!
+
+You might be wondering what I'm doing commenting here on your PR.
+
+**Yes, as a matter of fact, I am...**
+
+I'm just here to help us improve the documentation. I can't respond to
+questions or anything, but what I *can* do, I do well!
+
+**Okay... so what do you do?**
+
+I detect modules that are missing docstrings or "CLI Example" on existing docstrings!
+When I was created we had a *lot* of these. The documentation for these
+modules need some love and attention to make Salt better for our users.
+
+**So what does that have to do with my PR?**
+
+I noticed that in this PR there are some files changed that have some of these
+issues. So I'm leaving this comment to let you know your options.
+
+**Okay, what are they?**
+
+Well, my favorite, is that since you were making changes here I'm hoping that
+you would be the most familiar with this module and be able to add some other
+examples or fix any of the reported issues.
+
+**If I can, then what?**
+
+Well, you can either add them to this PR or add them to another PR. Either way is fine!
+
+**Well... what if I can't, or don't want to?**
+
+That's also fine! We appreciate *all* contributions to the Salt Project. If you
+can't add those other examples, either because you're too busy, or unfamiliar,
+or you just aren't interested, we still appreciate the contributions that
+you've made already.
+
+Whatever approach you decide to take, just drop a comment in the PR letting us know!
+"""
+
+
+def annotate(kind: str, fpath: str, start_lineno: int, end_lineno: int, message: str):
+ if kind not in ("warning", "error"):
+ raise RuntimeError("The annotation kind can only be one of 'warning', 'error'.")
+ if os.environ.get("GH_ACTIONS_ANNOTATE") is None:
+ return
+
+ github_output = os.environ.get("GITHUB_OUTPUT")
+ if github_output is None:
+ utils.warn("The 'GITHUB_OUTPUT' variable is not set. Not adding annotations.")
+ return
+
+ if TYPE_CHECKING:
+ assert github_output is not None
+
+ message = (
+ message.rstrip().replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A")
+ )
+ # Print it to stdout so that the GitHub runner pick's it up and adds the annotation
+ print(
+ f"::{kind} file={fpath},line={start_lineno},endLine={end_lineno}::{message}",
+ file=sys.stdout,
+ flush=True,
+ )
+
@task(iterable=["files"], positional=["files"])
def check(ctx, files, check_proper_formatting=False, error_on_known_failures=False):
@@ -839,6 +909,13 @@ def check(ctx, files, check_proper_formatting=False, error_on_known_failures=Fal
path.relative_to(CODE_DIR),
*error,
)
+ annotate(
+ "error",
+ path.relative_to(CODE_DIR),
+ funcdef.lineno,
+ funcdef.body[0].lineno,
+ "Version {1:r!} is not valid for {0!r}".format(*error),
+ )
if not str(path).startswith(SALT_INTERNAL_LOADERS_PATHS):
# No further docstrings checks are needed
@@ -863,6 +940,13 @@ def check(ctx, files, check_proper_formatting=False, error_on_known_failures=Fal
funcname,
relpath,
)
+ annotate(
+ "warning",
+ path.relative_to(CODE_DIR),
+ funcdef.lineno,
+ funcdef.body[0].lineno,
+ "Missing docstring",
+ )
continue
errors += 1
exitcode = 1
@@ -871,6 +955,13 @@ def check(ctx, files, check_proper_formatting=False, error_on_known_failures=Fal
funcname,
relpath,
)
+ annotate(
+ "error",
+ path.relative_to(CODE_DIR),
+ funcdef.lineno,
+ funcdef.body[0].lineno,
+ "Missing docstring",
+ )
continue
elif funcname in MISSING_DOCSTRINGS.get(relpath, ()):
# This was previously a know function with a missing docstring.
@@ -907,6 +998,13 @@ def check(ctx, files, check_proper_formatting=False, error_on_known_failures=Fal
funcname,
relpath,
)
+ annotate(
+ "warning",
+ path.relative_to(CODE_DIR),
+ funcdef.lineno,
+ funcdef.body[0].lineno,
+ "Missing 'CLI Example:' in docstring",
+ )
continue
errors += 1
exitcode = 1
@@ -915,6 +1013,13 @@ def check(ctx, files, check_proper_formatting=False, error_on_known_failures=Fal
funcname,
relpath,
)
+ annotate(
+ "error",
+ path.relative_to(CODE_DIR),
+ funcdef.lineno,
+ funcdef.body[0].lineno,
+ "Missing 'CLI Example:' in docstring",
+ )
continue
elif funcname in MISSING_EXAMPLES.get(relpath, ()):
# This was previously a know function with a missing CLI example
@@ -948,6 +1053,19 @@ def check(ctx, files, check_proper_formatting=False, error_on_known_failures=Fal
funcdef.name,
path.relative_to(CODE_DIR),
)
+ annotate(
+ "warning",
+ path.relative_to(CODE_DIR),
+ funcdef.lineno,
+ funcdef.body[0].lineno,
+ "Wrong format in 'CLI Example:' in docstring.\n"
+ "The proper format is:\n```"
+ "CLI Example:\n"
+ "\n"
+ ".. code-block:: bash\n"
+ "\n"
+ " salt '*' \n```",
+ )
continue
finally:
if contents != path.read_text():
@@ -957,6 +1075,11 @@ def check(ctx, files, check_proper_formatting=False, error_on_known_failures=Fal
utils.warn("Found {} warnings", warnings)
if exitcode:
utils.error("Found {} errors", errors)
+ if os.environ.get("GH_ACTIONS_ANNOTATE") and (warnings or errors):
+ github_step_summary = os.environ.get("GITHUB_STEP_SUMMARY")
+ if github_step_summary:
+ with open(github_step_summary, "w", encoding="utf-8") as wfh:
+ wfh.write(SUMMARY)
utils.exit_invoke(exitcode)