From c4a891d4251ee79554de581c7e6dc6af7da1ed21 Mon Sep 17 00:00:00 2001 From: Zach White Date: Sat, 18 Sep 2021 21:49:40 -0700 Subject: [PATCH 1/9] add mechanisms for finding and pinging maintainers --- .github/workflows/maintainers.yaml | 34 ++++++++++++++++++++++++++ lib/python/qmk/cli/__init__.py | 2 ++ lib/python/qmk/cli/list/maintainers.py | 16 ++++++++++++ lib/python/qmk/cli/ping/maintainers.py | 23 +++++++++++++++++ lib/python/qmk/maintainers.py | 22 +++++++++++++++++ 5 files changed, 97 insertions(+) create mode 100644 .github/workflows/maintainers.yaml create mode 100644 lib/python/qmk/cli/list/maintainers.py create mode 100644 lib/python/qmk/cli/ping/maintainers.py create mode 100644 lib/python/qmk/maintainers.py diff --git a/.github/workflows/maintainers.yaml b/.github/workflows/maintainers.yaml new file mode 100644 index 0000000000..ea1c47ca50 --- /dev/null +++ b/.github/workflows/maintainers.yaml @@ -0,0 +1,34 @@ +name: PR Lint Format + +on: + pull_request: + +jobs: + lint: + runs-on: ubuntu-latest + + container: qmkfm/qmk_cli + + steps: + - uses: rlespinasse/github-slug-action@v3.x + + - uses: actions/checkout@v2 + with: + fetch-depth: 0 + + - uses: trilom/file-changes-action@v1.2.4 + id: file_changes + with: + output: " " + fileOutput: " " + + - name: Get our ping message + shell: "bash {0}" + run: echo "ping_message=$(qmk ping-maintainers $(< ~/files.txt))" > $GITHUB_ENV + + - uses: mshick/add-pr-comment@v1 + with: + message: ${{ env.ping_message }} + repo-token: ${{ secrets.GITHUB_TOKEN }} + repo-token-user-login: "github-actions[bot]" # The user.login for temporary GitHub tokens + allow-repeats: false # This is the default diff --git a/lib/python/qmk/cli/__init__.py b/lib/python/qmk/cli/__init__.py index f45e33240c..49f32b2965 100644 --- a/lib/python/qmk/cli/__init__.py +++ b/lib/python/qmk/cli/__init__.py @@ -60,10 +60,12 @@ subcommands = [ 'qmk.cli.lint', 'qmk.cli.list.keyboards', 'qmk.cli.list.keymaps', + 'qmk.cli.list.maintainers', 'qmk.cli.kle2json', 'qmk.cli.multibuild', 'qmk.cli.new.keyboard', 'qmk.cli.new.keymap', + 'qmk.cli.ping.maintainers', 'qmk.cli.pyformat', 'qmk.cli.pytest', ] diff --git a/lib/python/qmk/cli/list/maintainers.py b/lib/python/qmk/cli/list/maintainers.py new file mode 100644 index 0000000000..a9186451b3 --- /dev/null +++ b/lib/python/qmk/cli/list/maintainers.py @@ -0,0 +1,16 @@ +"""List the keymaps for a specific keyboard +""" +from pathlib import Path + +from milc import cli + +from qmk.maintainers import maintainers + + +@cli.argument("files", type=Path, arg_only=True, nargs='*', help="File to check") +@cli.subcommand("List the maintainers for a file.") +def list_maintainers(cli): + """List the maintainers for a file. + """ + for file in cli.args.files: + cli.echo('%s: %s', file, ', '.join(maintainers(file))) diff --git a/lib/python/qmk/cli/ping/maintainers.py b/lib/python/qmk/cli/ping/maintainers.py new file mode 100644 index 0000000000..77a333b43d --- /dev/null +++ b/lib/python/qmk/cli/ping/maintainers.py @@ -0,0 +1,23 @@ +"""Generate a message to ping people responsible for one or more files. +""" +from pathlib import Path + +from milc import cli + +from qmk.maintainers import maintainers + + +@cli.argument("files", type=Path, arg_only=True, nargs='*', help="File to ping maintainers for.") +@cli.subcommand("Ping the maintainers for one or more files.") +def ping_maintainers(cli): + """List the maintainers for one or more files. + """ + github_maintainers = set() + + for file in cli.args.files: + for maintainer in maintainers(file): + if maintainer != 'qmk/collaborators': + github_maintainers.add('@' + maintainer) + + if github_maintainers: + print(f'If you were pinged by this comment you have one or more files being changed by this PR: {" ".join(sorted(github_maintainers))}') diff --git a/lib/python/qmk/maintainers.py b/lib/python/qmk/maintainers.py new file mode 100644 index 0000000000..bfcb789602 --- /dev/null +++ b/lib/python/qmk/maintainers.py @@ -0,0 +1,22 @@ +from pathlib import Path + +from qmk.json_schema import json_load + + +def maintainers(file): + """Yields maintainers for a file. + """ + maintainers = 'qmk' + file_dir = file if file.is_dir() else file.parent + + cur_path = Path('.') + + for path_part in file_dir.parts: + cur_path = cur_path / path_part + info_file = cur_path / 'info.json' + if info_file.exists(): + new_info_data = json_load(info_file) + maintainers = new_info_data.get('maintainer', maintainers) + + for maintainer in maintainers.replace(',', ' ').split(): + yield 'qmk/collaborators' if maintainer == 'qmk' else maintainer \ No newline at end of file From 65fd33ec0e55a0bf883cbb2c9df6a1d340ed5e5e Mon Sep 17 00:00:00 2001 From: Zach White Date: Sat, 18 Sep 2021 22:51:44 -0700 Subject: [PATCH 2/9] Add CODEOWNERS support --- CODEOWNERS | 2 ++ lib/python/qmk/cli/ping/maintainers.py | 2 +- lib/python/qmk/maintainers.py | 14 ++++++++++---- requirements-dev.txt | 1 + 4 files changed, 14 insertions(+), 5 deletions(-) create mode 100644 CODEOWNERS diff --git a/CODEOWNERS b/CODEOWNERS new file mode 100644 index 0000000000..63a17be22c --- /dev/null +++ b/CODEOWNERS @@ -0,0 +1,2 @@ +* @qmk/collaborators +/lib/python/* @skullydazed @erovia diff --git a/lib/python/qmk/cli/ping/maintainers.py b/lib/python/qmk/cli/ping/maintainers.py index 77a333b43d..83bc6b224d 100644 --- a/lib/python/qmk/cli/ping/maintainers.py +++ b/lib/python/qmk/cli/ping/maintainers.py @@ -17,7 +17,7 @@ def ping_maintainers(cli): for file in cli.args.files: for maintainer in maintainers(file): if maintainer != 'qmk/collaborators': - github_maintainers.add('@' + maintainer) + github_maintainers.add(maintainer) if github_maintainers: print(f'If you were pinged by this comment you have one or more files being changed by this PR: {" ".join(sorted(github_maintainers))}') diff --git a/lib/python/qmk/maintainers.py b/lib/python/qmk/maintainers.py index bfcb789602..011660166a 100644 --- a/lib/python/qmk/maintainers.py +++ b/lib/python/qmk/maintainers.py @@ -1,12 +1,17 @@ from pathlib import Path +from codeowners import CodeOwners + from qmk.json_schema import json_load +codeowners_file = Path('CODEOWNERS') +codeowners = CodeOwners(codeowners_file.read_text()) + def maintainers(file): """Yields maintainers for a file. """ - maintainers = 'qmk' + maintainers = [owner[1] for owner in codeowners.of(str(file))] file_dir = file if file.is_dir() else file.parent cur_path = Path('.') @@ -16,7 +21,8 @@ def maintainers(file): info_file = cur_path / 'info.json' if info_file.exists(): new_info_data = json_load(info_file) - maintainers = new_info_data.get('maintainer', maintainers) + maintainers = new_info_data.get('maintainer').replace(',', ' ').split() + maintainers = ['@' + maintainer for maintainer in maintainers] - for maintainer in maintainers.replace(',', ' ').split(): - yield 'qmk/collaborators' if maintainer == 'qmk' else maintainer \ No newline at end of file + for maintainer in maintainers: + yield '@qmk/collaborators' if maintainer == 'qmk' else maintainer \ No newline at end of file diff --git a/requirements-dev.txt b/requirements-dev.txt index 1db3b6d733..be6082dfed 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -2,6 +2,7 @@ -r requirements.txt # Python development requirements +codeowners nose2 flake8 pep8-naming From 405e200ca2e9201edecda505881bf9f95df98361 Mon Sep 17 00:00:00 2001 From: Zach White Date: Sat, 18 Sep 2021 22:57:07 -0700 Subject: [PATCH 3/9] install deps --- .github/workflows/maintainers.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/maintainers.yaml b/.github/workflows/maintainers.yaml index ea1c47ca50..c5cb88eeaf 100644 --- a/.github/workflows/maintainers.yaml +++ b/.github/workflows/maintainers.yaml @@ -10,8 +10,6 @@ jobs: container: qmkfm/qmk_cli steps: - - uses: rlespinasse/github-slug-action@v3.x - - uses: actions/checkout@v2 with: fetch-depth: 0 @@ -22,6 +20,9 @@ jobs: output: " " fileOutput: " " + - name: Install dependencies + run: pip3 install -r requirements-dev.txt + - name: Get our ping message shell: "bash {0}" run: echo "ping_message=$(qmk ping-maintainers $(< ~/files.txt))" > $GITHUB_ENV From 771c47ab99f164afbed5d9bd26a75c1ee5e11f1b Mon Sep 17 00:00:00 2001 From: Zach White Date: Sat, 18 Sep 2021 23:04:10 -0700 Subject: [PATCH 4/9] refine how we ping --- CODEOWNERS | 2 +- lib/python/qmk/cli/ping/maintainers.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CODEOWNERS b/CODEOWNERS index 63a17be22c..2518b6475a 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -1,2 +1,2 @@ * @qmk/collaborators -/lib/python/* @skullydazed @erovia +/lib/python/* @qmk/python diff --git a/lib/python/qmk/cli/ping/maintainers.py b/lib/python/qmk/cli/ping/maintainers.py index 83bc6b224d..89faaf3b27 100644 --- a/lib/python/qmk/cli/ping/maintainers.py +++ b/lib/python/qmk/cli/ping/maintainers.py @@ -16,7 +16,7 @@ def ping_maintainers(cli): for file in cli.args.files: for maintainer in maintainers(file): - if maintainer != 'qmk/collaborators': + if not maintainer.startswith('@qmk/'): github_maintainers.add(maintainer) if github_maintainers: From eee765d454653b7703fc8cbf9aa51af0945ffa2f Mon Sep 17 00:00:00 2001 From: Zach White Date: Sat, 18 Sep 2021 23:10:37 -0700 Subject: [PATCH 5/9] add missing newline --- lib/python/qmk/maintainers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/python/qmk/maintainers.py b/lib/python/qmk/maintainers.py index 011660166a..ccde8c07fb 100644 --- a/lib/python/qmk/maintainers.py +++ b/lib/python/qmk/maintainers.py @@ -25,4 +25,4 @@ def maintainers(file): maintainers = ['@' + maintainer for maintainer in maintainers] for maintainer in maintainers: - yield '@qmk/collaborators' if maintainer == 'qmk' else maintainer \ No newline at end of file + yield '@qmk/collaborators' if maintainer == 'qmk' else maintainer From 40e8ab382ced66a8b977f11519539bc72a9720b6 Mon Sep 17 00:00:00 2001 From: Zach White Date: Sat, 18 Sep 2021 23:11:30 -0700 Subject: [PATCH 6/9] tweak the maintainers job --- .github/workflows/maintainers.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/maintainers.yaml b/.github/workflows/maintainers.yaml index c5cb88eeaf..4283f8af0a 100644 --- a/.github/workflows/maintainers.yaml +++ b/.github/workflows/maintainers.yaml @@ -1,10 +1,10 @@ -name: PR Lint Format +name: Ping maintainers on: pull_request: jobs: - lint: + maintainer_ping: runs-on: ubuntu-latest container: qmkfm/qmk_cli From 1db04e4622c950b3fe490b0f27d9a506e2bc5b37 Mon Sep 17 00:00:00 2001 From: Zach White Date: Sat, 18 Sep 2021 23:15:53 -0700 Subject: [PATCH 7/9] delay importing of codeowners --- lib/python/qmk/maintainers.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/python/qmk/maintainers.py b/lib/python/qmk/maintainers.py index ccde8c07fb..c851c34fb9 100644 --- a/lib/python/qmk/maintainers.py +++ b/lib/python/qmk/maintainers.py @@ -1,16 +1,15 @@ from pathlib import Path -from codeowners import CodeOwners - from qmk.json_schema import json_load -codeowners_file = Path('CODEOWNERS') -codeowners = CodeOwners(codeowners_file.read_text()) - def maintainers(file): """Yields maintainers for a file. """ + from codeowners import CodeOwners + + codeowners_file = Path('CODEOWNERS') + codeowners = CodeOwners(codeowners_file.read_text()) maintainers = [owner[1] for owner in codeowners.of(str(file))] file_dir = file if file.is_dir() else file.parent From 610111583b4fccdd996021a943f8b02f200aca25 Mon Sep 17 00:00:00 2001 From: Zach White Date: Tue, 21 Sep 2021 14:28:28 -0700 Subject: [PATCH 8/9] add the pr comment ourselves and request reviews --- .github/workflows/maintainers.yaml | 11 ++------ lib/python/qmk/cli/ping/maintainers.py | 38 ++++++++++++++++++++++---- requirements-dev.txt | 3 +- 3 files changed, 37 insertions(+), 15 deletions(-) diff --git a/.github/workflows/maintainers.yaml b/.github/workflows/maintainers.yaml index 4283f8af0a..857be0b416 100644 --- a/.github/workflows/maintainers.yaml +++ b/.github/workflows/maintainers.yaml @@ -23,13 +23,6 @@ jobs: - name: Install dependencies run: pip3 install -r requirements-dev.txt - - name: Get our ping message + - name: Ping maintainers and request reviews shell: "bash {0}" - run: echo "ping_message=$(qmk ping-maintainers $(< ~/files.txt))" > $GITHUB_ENV - - - uses: mshick/add-pr-comment@v1 - with: - message: ${{ env.ping_message }} - repo-token: ${{ secrets.GITHUB_TOKEN }} - repo-token-user-login: "github-actions[bot]" # The user.login for temporary GitHub tokens - allow-repeats: false # This is the default + run: qmk ping-maintainers --pr ${{ github.event.number }} ${{ steps.file_changes.outputs.files }} diff --git a/lib/python/qmk/cli/ping/maintainers.py b/lib/python/qmk/cli/ping/maintainers.py index 89faaf3b27..4af7520588 100644 --- a/lib/python/qmk/cli/ping/maintainers.py +++ b/lib/python/qmk/cli/ping/maintainers.py @@ -7,17 +7,45 @@ from milc import cli from qmk.maintainers import maintainers +@cli.argument('--pr', type=int, arg_only=True, help="PR to send ping to (optional)") +@cli.argument('--owner', default='qmk', arg_only=True, help="Owner for the repo (Default: qmk)") +@cli.argument('--repo', default='qmk_firmware', arg_only=True, help="Repo to send pings to (Default: qmk_firmware)") @cli.argument("files", type=Path, arg_only=True, nargs='*', help="File to ping maintainers for.") -@cli.subcommand("Ping the maintainers for one or more files.") +@cli.subcommand("Ping the maintainers and request reviews for one or more files.") def ping_maintainers(cli): - """List the maintainers for one or more files. + """Ping the maintainers for one or more files. """ github_maintainers = set() + github_teams = set() for file in cli.args.files: for maintainer in maintainers(file): - if not maintainer.startswith('@qmk/'): + if '/' in maintainer: + github_teams.add(maintainer) + else: github_maintainers.add(maintainer) - if github_maintainers: - print(f'If you were pinged by this comment you have one or more files being changed by this PR: {" ".join(sorted(github_maintainers))}') + if cli.args.pr: + from ghapi.all import GhApi + + ghapi = GhApi(owner=cli.args.owner, repo=cli.args.repo) + pr = ghapi.pulls(cli.args.pr) + + if not pr.draft: + for team in pr.requested_teams: + team_name = f'@{cli.args.owner}/{team.slug}' + + if team_name in github_teams: + cli.log.info('Found %s in reviews already, skipping', team_name) + github_teams.remove(team_name) + + for team in github_teams: + cli.log.info('Requesting review from team %s', team.split('/', 1)[1]) + ghapi.pulls.request_reviewers(pull_number=cli.args.pr, team_reviewers=team.split('/', 1)[1]) + + if github_maintainers: + ghapi.issues.create_comment(cli.args.pr, f'If you were pinged by this comment you have one or more files being changed by this PR: {" ".join(sorted(github_maintainers))}') + + else: + print(f'Team Reviews: {" ".join(sorted(github_teams))}') + print(f'Individual Reviews: {" ".join(sorted(github_maintainers))}') diff --git a/requirements-dev.txt b/requirements-dev.txt index be6082dfed..a5d59a3d54 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -3,7 +3,8 @@ # Python development requirements codeowners -nose2 flake8 +ghapi +nose2 pep8-naming yapf From 295cb5c60a8dbf370f188972eb037ecfe376d7bc Mon Sep 17 00:00:00 2001 From: Zach White Date: Tue, 21 Sep 2021 14:33:59 -0700 Subject: [PATCH 9/9] fix stacktrace --- lib/python/qmk/cli/ping/maintainers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/python/qmk/cli/ping/maintainers.py b/lib/python/qmk/cli/ping/maintainers.py index 4af7520588..c8ce636c1a 100644 --- a/lib/python/qmk/cli/ping/maintainers.py +++ b/lib/python/qmk/cli/ping/maintainers.py @@ -29,7 +29,7 @@ def ping_maintainers(cli): from ghapi.all import GhApi ghapi = GhApi(owner=cli.args.owner, repo=cli.args.repo) - pr = ghapi.pulls(cli.args.pr) + pr = ghapi.pulls.get(cli.args.pr) if not pr.draft: for team in pr.requested_teams: