From ad6cc29b0526d224d9955835a30ca105a41de36a Mon Sep 17 00:00:00 2001 From: Maksym H Date: Tue, 10 Sep 2024 13:09:04 +0100 Subject: [PATCH 1/4] add prdoc to /cmd --- .github/scripts/cmd/cmd.py | 24 +++++++++- .github/scripts/cmd/test_cmd.py | 18 +++++++ .github/scripts/generate-prdoc.py | 24 +++++++--- .../scripts/generate-prdoc.requirements.txt | 6 +++ .github/workflows/cmd.yml | 48 +++++++++---------- docs/contributor/commands-readme.md | 5 +- docs/contributor/prdoc.md | 25 ++++++---- 7 files changed, 109 insertions(+), 41 deletions(-) create mode 100644 .github/scripts/generate-prdoc.requirements.txt diff --git a/.github/scripts/cmd/cmd.py b/.github/scripts/cmd/cmd.py index 1c08b621467d..65f2e711dd79 100755 --- a/.github/scripts/cmd/cmd.py +++ b/.github/scripts/cmd/cmd.py @@ -5,14 +5,13 @@ import json import argparse import _help +import importlib.util _HelpAction = _help._HelpAction f = open('.github/workflows/runtimes-matrix.json', 'r') runtimesMatrix = json.load(f) -print(f'runtimesMatrix: {runtimesMatrix}\n') - runtimeNames = list(map(lambda x: x['name'], runtimesMatrix)) common_args = { @@ -69,6 +68,20 @@ for arg, config in common_args.items(): parser_ui.add_argument(arg, **config) +""" +PRDOC +""" +# Import generate-prdoc.py dynamically +spec = importlib.util.spec_from_file_location("generate_prdoc", ".github/scripts/generate-prdoc.py") +generate_prdoc = importlib.util.module_from_spec(spec) +spec.loader.exec_module(generate_prdoc) + +# Create the prdoc subparser +parser_prdoc = subparsers.add_parser('prdoc', help='Generates PR documentation') + +# Use parse_args from generate_prdoc to add arguments +generate_prdoc.parse_args(parser_prdoc) + def main(): global args, unknown, runtimesMatrix args, unknown = parser.parse_known_args() @@ -215,6 +228,13 @@ def main(): print('❌ Failed to format code') sys.exit(1) + elif args.command == 'prdoc': + # Call the main function from generate_prdoc module + exit_code = generate_prdoc.main(args) + if exit_code != 0 and not args.continue_on_fail: + print('❌ Failed to generate prdoc') + sys.exit(exit_code) + print('🚀 Done') if __name__ == '__main__': diff --git a/.github/scripts/cmd/test_cmd.py b/.github/scripts/cmd/test_cmd.py index 4cf1b290915d..a2f29b075dae 100644 --- a/.github/scripts/cmd/test_cmd.py +++ b/.github/scripts/cmd/test_cmd.py @@ -31,12 +31,18 @@ def setUp(self): self.patcher3 = patch('argparse.ArgumentParser.parse_known_args') self.patcher4 = patch('os.system', return_value=0) self.patcher5 = patch('os.popen') + self.patcher6 = patch('importlib.util.spec_from_file_location', return_value=MagicMock()) + self.patcher7 = patch('importlib.util.module_from_spec', return_value=MagicMock()) + self.patcher8 = patch('cmd.generate_prdoc.main', return_value=0) self.mock_open = self.patcher1.start() self.mock_json_load = self.patcher2.start() self.mock_parse_args = self.patcher3.start() self.mock_system = self.patcher4.start() self.mock_popen = self.patcher5.start() + self.mock_spec_from_file_location = self.patcher6.start() + self.mock_module_from_spec = self.patcher7.start() + self.mock_generate_prdoc_main = self.patcher8.start() # Ensure that cmd.py uses the mock_runtimes_matrix import cmd @@ -48,6 +54,9 @@ def tearDown(self): self.patcher3.stop() self.patcher4.stop() self.patcher5.stop() + self.patcher6.stop() + self.patcher7.stop() + self.patcher8.stop() def test_bench_command_normal_execution_all_runtimes(self): self.mock_parse_args.return_value = (argparse.Namespace( @@ -317,5 +326,14 @@ def test_update_ui_command(self, mock_system, mock_parse_args): mock_exit.assert_not_called() mock_system.assert_called_with('sh ./scripts/update-ui-tests.sh') + @patch('argparse.ArgumentParser.parse_known_args', return_value=(argparse.Namespace(command='prdoc', continue_on_fail=False), [])) + @patch('os.system', return_value=0) + def test_prdoc_command(self, mock_system, mock_parse_args): + with patch('sys.exit') as mock_exit: + import cmd + cmd.main() + mock_exit.assert_not_called() + self.mock_generate_prdoc_main.assert_called_with(mock_parse_args.return_value[0]) + if __name__ == '__main__': unittest.main() \ No newline at end of file diff --git a/.github/scripts/generate-prdoc.py b/.github/scripts/generate-prdoc.py index ba7def20fcb9..cebb89990495 100644 --- a/.github/scripts/generate-prdoc.py +++ b/.github/scripts/generate-prdoc.py @@ -98,16 +98,28 @@ def create_prdoc(pr, audience, title, description, patch, bump, force): yaml.dump(prdoc, f) print(f"PrDoc for PR {pr} written to {path}") -def parse_args(): - parser = argparse.ArgumentParser() +# parse_args is also used by cmd/cmd.py +def parse_args(parser=None): + if parser is None: + parser = argparse.ArgumentParser() + parser.add_argument("--pr", type=int, required=True) parser.add_argument("--audience", type=str, default="TODO") parser.add_argument("--bump", type=str, default="TODO") parser.add_argument("--force", type=str) - return parser.parse_args() + + return parser -if __name__ == "__main__": - args = parse_args() +def main(args): force = True if args.force.lower() == "true" else False print(f"Args: {args}, force: {force}") - from_pr_number(args.pr, args.audience, args.bump, force) + try: + from_pr_number(args.pr, args.audience, args.bump, force) + return 0 + except Exception as e: + print(f"Error generating prdoc: {e}") + return 1 + +if __name__ == "__main__": + args = parse_args().parse_args() + main(args) diff --git a/.github/scripts/generate-prdoc.requirements.txt b/.github/scripts/generate-prdoc.requirements.txt new file mode 100644 index 000000000000..c17aceff63a0 --- /dev/null +++ b/.github/scripts/generate-prdoc.requirements.txt @@ -0,0 +1,6 @@ +requests +cargo-workspace +PyGithub +whatthepatch +pyyaml +toml \ No newline at end of file diff --git a/.github/workflows/cmd.yml b/.github/workflows/cmd.yml index 79a4f6c3b19c..d01691665f7d 100644 --- a/.github/workflows/cmd.yml +++ b/.github/workflows/cmd.yml @@ -2,7 +2,7 @@ name: Command on: issue_comment: # listen for comments on issues - types: [ created ] + types: [created] permissions: # allow the action to comment on the PR contents: write @@ -35,14 +35,14 @@ jobs: try { const org = '${{ github.event.repository.owner.login }}'; const username = '${{ github.event.comment.user.login }}'; - + const membership = await github.rest.orgs.checkMembershipForUser({ org: org, username: username }); - + console.log(membership, membership.status, membership.status === 204); - + if (membership.status === 204) { return 'true'; } else { @@ -52,7 +52,7 @@ jobs: } catch (error) { console.log(error) } - + return 'false'; reject-non-members: @@ -140,7 +140,7 @@ jobs: } }) help: - needs: [ clean, is-org-member ] + needs: [clean, is-org-member] if: ${{ startsWith(github.event.comment.body, '/cmd') && contains(github.event.comment.body, '--help') && needs.is-org-member.outputs.member == 'true' }} runs-on: ubuntu-latest steps: @@ -173,11 +173,11 @@ jobs: owner: context.repo.owner, repo: context.repo.repo, body: `
Command help: - + \`\`\` ${{ steps.help.outputs.help }} \`\`\` - +
` }) @@ -208,7 +208,7 @@ jobs: }) set-image: - needs: [ clean, is-org-member ] + needs: [clean, is-org-member] if: ${{ startsWith(github.event.comment.body, '/cmd') && !contains(github.event.comment.body, '--help') && needs.is-org-member.outputs.member == 'true' }} runs-on: ubuntu-latest outputs: @@ -222,13 +222,13 @@ jobs: run: | BODY=$(echo "${{ github.event.comment.body }}" | xargs) IMAGE_OVERRIDE=$(echo $BODY | grep -oe 'docker.io/paritytech/ci-unified:.*\s' | xargs) - + cat .github/env >> $GITHUB_OUTPUT - + if [ -n "$IMAGE_OVERRIDE" ]; then echo "IMAGE=$IMAGE_OVERRIDE" >> $GITHUB_OUTPUT fi - + if [[ $BODY == "/cmd bench"* ]]; then echo "RUNNER=arc-runners-polkadot-sdk-benchmark" >> $GITHUB_OUTPUT elif [[ $BODY == "/cmd update-ui"* ]]; then @@ -239,7 +239,7 @@ jobs: # Get PR branch name, because the issue_comment event does not contain the PR branch name get-pr-branch: - needs: [ set-image ] + needs: [set-image] runs-on: ubuntu-latest outputs: pr-branch: ${{ steps.get-pr.outputs.pr_branch }} @@ -278,9 +278,9 @@ jobs: echo "The repository is ${{ steps.get-pr.outputs.repo }}" cmd: - needs: [ set-image, get-pr-branch ] + needs: [set-image, get-pr-branch] env: - JOB_NAME: 'cmd' + JOB_NAME: "cmd" runs-on: ${{ needs.set-image.outputs.RUNNER }} container: image: ${{ needs.set-image.outputs.IMAGE }} @@ -301,18 +301,17 @@ jobs: -H "Authorization: token ${{ secrets.GITHUB_TOKEN }}" \ -H "Accept: application/vnd.github.v3+json" \ https://api.github.com/repos/${{ github.repository }}/actions/runs/${{ github.run_id }}/jobs | jq '.jobs[] | select(.name | contains("${{ env.JOB_NAME }}")) | .html_url') - + runLink=$(curl -s \ -H "Authorization: token ${{ secrets.GITHUB_TOKEN }}" \ -H "Accept: application/vnd.github.v3+json" \ https://api.github.com/repos/${{ github.repository }}/actions/runs/${{ github.run_id }} | jq '.html_url') - + echo "job_url=${jobLink}" echo "run_url=${runLink}" echo "job_url=$jobLink" >> $GITHUB_OUTPUT echo "run_url=$runLink" >> $GITHUB_OUTPUT - - name: Comment PR (Start) if: ${{ !contains(github.event.comment.body, '--quiet') }} uses: actions/github-script@v7 @@ -320,7 +319,7 @@ jobs: github-token: ${{ secrets.GITHUB_TOKEN }} script: | let job_url = ${{ steps.build-link.outputs.job_url }} - + github.rest.issues.createComment({ issue_number: context.issue.number, owner: context.repo.owner, @@ -348,6 +347,7 @@ jobs: # Fixes "detected dubious ownership" error in the ci git config --global --add safe.directory '*' git remote -v + python3 -m pip install -r .github/scripts/generate-prdoc.requirements.txt python3 .github/scripts/cmd/cmd.py $CMD git status git diff @@ -357,7 +357,7 @@ jobs: if [ -n "$(git status --porcelain)" ]; then git config --local user.email "action@github.com" git config --local user.name "GitHub Action" - + git pull --rebase origin ${{ needs.get-pr-branch.outputs.pr-branch }} git add . git restore --staged Cargo.lock # ignore changes in Cargo.lock @@ -381,7 +381,7 @@ jobs: --change added changed \ --ignore-errors \ refs/remotes/origin/master refs/heads/${{ needs.get-pr-branch.outputs.pr-branch }}) - + # Save the multiline result to the output { echo "result<\n\nSubweight results:\n\n${subweight}\n\n` : ''; - + github.rest.issues.createComment({ issue_number: context.issue.number, owner: context.repo.owner, diff --git a/docs/contributor/commands-readme.md b/docs/contributor/commands-readme.md index 2bb9bd7e7d58..861c3ac784d5 100644 --- a/docs/contributor/commands-readme.md +++ b/docs/contributor/commands-readme.md @@ -9,13 +9,14 @@ Note: it works only for members of the `paritytech` organization. `/cmd --help` to see the usage of a specific command - ### Commands - `/cmd fmt` to format the code in the PR. It commits back with the formatted code (fmt) and configs (taplo). - `/cmd bench` to generate weights for a runtime. Read more about [Weight Generation](weight-generation.md) +- `/cmd prdoc` to generate a prdoc for a PR. Read more about [PRDoc](prdoc.md) + ### Flags 1.`--quiet` to suppress the output of the command in the comments. @@ -32,12 +33,14 @@ The pipeline logs will include what is failed (like which runtimes/pallets), the or they keep failing, and you're rerunning them again, it's handy to add this flag to keep a PR clean. ### Adding new Commands + Feel free to add new commands to the workflow, however **_note_** that triggered workflows will use the actions from `main` (default) branch, meaning they will take effect only after the PR with new changes/command is merged. If you want to test the new command, it's better to test in your fork and local-to-fork PRs, where you control the default branch. ### Examples + The regex in cmd.yml is: `^(\/cmd )([-\/\s\w.=:]+)$` accepts only alphanumeric, space, "-", "/", "=", ":", "." chars. `/cmd bench --runtime bridge-hub-westend --pallet=pallet_name` diff --git a/docs/contributor/prdoc.md b/docs/contributor/prdoc.md index 0c8165af40f4..a17546914e3e 100644 --- a/docs/contributor/prdoc.md +++ b/docs/contributor/prdoc.md @@ -18,24 +18,33 @@ A `.prdoc` file is a YAML file with a defined structure (ie JSON Schema). Please 1. Open a Pull Request and get the PR number. 1. Generate the file with `prdoc generate `. The output filename will be printed. 1. Optional: Install the `prdoc/schema_user.json` schema in your editor, for example -[VsCode](https://github.com/paritytech/prdoc?tab=readme-ov-file#schemas). + [VsCode](https://github.com/paritytech/prdoc?tab=readme-ov-file#schemas). 1. Edit your `.prdoc` file according to the [Audience](#pick-an-audience) and [SemVer](#record-semver-changes) sections. 1. Check your prdoc with `prdoc check -n `. This is optional since the CI will also check it. > **Tip:** GitHub CLI and jq can be used to provide the number of your PR to generate the correct file: > `prdoc generate $(gh pr view --json number | jq '.number') -o prdoc` +Alternatively you can call the prdoc from PR via `/cmd prdoc` (see args with `/cmd prdoc --help`) in a comment to PR to trigger it from CI. + +Options: + +- `pr`: The PR number to generate the PrDoc for. +- `audience`: The audience of whom the changes may concern. +- `bump`: A default bump level for all crates. The PrDoc will likely need to be edited to reflect the actual changes after generation. +- `overwrite`: Whether to overwrite any existing PrDoc. + ## Pick An Audience While describing a PR, the author needs to consider which audience(s) need to be addressed. The list of valid audiences is described and documented in the JSON schema as follow: - `Node Dev`: Those who build around the client side code. Alternative client builders, SMOLDOT, those who consume RPCs. - These are people who are oblivious to the runtime changes. They only care about the meta-protocol, not the protocol - itself. + These are people who are oblivious to the runtime changes. They only care about the meta-protocol, not the protocol + itself. - `Runtime Dev`: All of those who rely on the runtime. A parachain team that is using a pallet. A DApp that is using a - pallet. These are people who care about the protocol (WASM), not the meta-protocol (client). + pallet. These are people who care about the protocol (WASM), not the meta-protocol (client). - `Node Operator`: Those who don't write any code and only run code. @@ -64,10 +73,10 @@ For example when you modified two crates and record the changes: ```yaml crates: -- name: frame-example - bump: major -- name: frame-example-pallet - bump: minor + - name: frame-example + bump: major + - name: frame-example-pallet + bump: minor ``` It means that downstream code using `frame-example-pallet` is still guaranteed to work as before, while code using From b63f442427f54c7e4fef72d764d2b4ad27a3b5e9 Mon Sep 17 00:00:00 2001 From: Maksym H Date: Tue, 10 Sep 2024 13:15:17 +0100 Subject: [PATCH 2/4] Update cmd.py --- .github/scripts/cmd/cmd.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/.github/scripts/cmd/cmd.py b/.github/scripts/cmd/cmd.py index 65f2e711dd79..2bbc3220f833 100755 --- a/.github/scripts/cmd/cmd.py +++ b/.github/scripts/cmd/cmd.py @@ -76,10 +76,7 @@ generate_prdoc = importlib.util.module_from_spec(spec) spec.loader.exec_module(generate_prdoc) -# Create the prdoc subparser parser_prdoc = subparsers.add_parser('prdoc', help='Generates PR documentation') - -# Use parse_args from generate_prdoc to add arguments generate_prdoc.parse_args(parser_prdoc) def main(): @@ -229,7 +226,7 @@ def main(): sys.exit(1) elif args.command == 'prdoc': - # Call the main function from generate_prdoc module + # Call the main function from ./github/scripts/generate-prdoc.py module exit_code = generate_prdoc.main(args) if exit_code != 0 and not args.continue_on_fail: print('❌ Failed to generate prdoc') From b713ea5455a0b55bb076601dabfaf14b03fbb2cf Mon Sep 17 00:00:00 2001 From: Maksym H Date: Tue, 10 Sep 2024 14:34:48 +0100 Subject: [PATCH 3/4] Update cmd.yml --- .github/workflows/cmd.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/cmd.yml b/.github/workflows/cmd.yml index d01691665f7d..f13a8ff09fa0 100644 --- a/.github/workflows/cmd.yml +++ b/.github/workflows/cmd.yml @@ -146,6 +146,8 @@ jobs: steps: - name: Checkout uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.sha }} - name: Get command uses: actions-ecosystem/action-regex-match@v2 @@ -330,8 +332,7 @@ jobs: - name: Checkout uses: actions/checkout@v4 with: - repository: ${{ needs.get-pr-branch.outputs.repo }} - ref: ${{ needs.get-pr-branch.outputs.pr-branch }} + ref: ${{ github.event.pull_request.head.sha }} - name: Install dependencies for bench if: startsWith(steps.get-pr-comment.outputs.group2, 'bench') From 77dad4fb46959d0578306e532b54dda7c3840e12 Mon Sep 17 00:00:00 2001 From: Maksym H Date: Tue, 10 Sep 2024 15:07:56 +0100 Subject: [PATCH 4/4] followups --- .github/scripts/generate-prdoc.py | 4 ++-- docs/contributor/prdoc.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/scripts/generate-prdoc.py b/.github/scripts/generate-prdoc.py index ace70fc44393..451e1b8fa01b 100644 --- a/.github/scripts/generate-prdoc.py +++ b/.github/scripts/generate-prdoc.py @@ -114,7 +114,7 @@ def yaml_multiline_string_presenter(dumper, data): yaml.add_representer(str, yaml_multiline_string_presenter) # parse_args is also used by cmd/cmd.py -def parse_args(parser=None): +def setup_parser(parser=None): if parser is None: parser = argparse.ArgumentParser() parser.add_argument("--pr", type=int, required=True) @@ -136,5 +136,5 @@ def main(args): return 1 if __name__ == "__main__": - args = parse_args().parse_args() + args = setup_parser().parse_args() main(args) \ No newline at end of file diff --git a/docs/contributor/prdoc.md b/docs/contributor/prdoc.md index f3abeb48a3d6..826158a24be9 100644 --- a/docs/contributor/prdoc.md +++ b/docs/contributor/prdoc.md @@ -32,7 +32,7 @@ Options: - `pr`: The PR number to generate the PrDoc for. - `audience`: The audience of whom the changes may concern. - `bump`: A default bump level for all crates. The PrDoc will likely need to be edited to reflect the actual changes after generation. -- `overwrite`: Whether to overwrite any existing PrDoc. +- `force`: Whether to overwrite any existing PrDoc. ## Pick An Audience