chore(ci): correct github-script API calls by dnwe · Pull Request #14442 · SeleniumHQ/selenium

dnwe · 2024-08-27T12:30:43Z

User description

Description

Since V5 of github-script the Octokit context available via github no longer has REST methods directly on it, they were moved to github.rest.* instead. Update the references in delete-comments.yml job to match.

Motivation and Context

The GitHub Actions workflow will fail when it attempts to delete a comment without this change.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist

I have read the contributing document.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.

PR Type

Bug fix, Enhancement

Description

Updated the GitHub Actions workflow to use the new github.rest.* syntax for API calls, ensuring compatibility with version 5 of github-script.
Modified the deleteComment and block methods to align with the updated Octokit context, preventing workflow failures.

Changes walkthrough 📝

Relevant files

Bug fix

delete-comments.yml `Update GitHub API calls to new Octokit syntax` .github/workflows/delete-comments.yml Updated GitHub API calls to use `github.rest.*` syntax. Changed `deleteComment` and `block` methods to use the new Octokit context.	+2/-2

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

CLAassistant · 2024-08-27T12:30:52Z

All committers have signed the CLA.

Since V5 of github-script the Octokit context available via `github` no longer has REST methods directly on it, they were moved to `github.rest.*` instead. Update the references in delete-comments.yml job to match. Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>

qodo-code-review · 2024-08-27T12:31:02Z

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ No key issues to review

qodo-code-review · 2024-08-27T12:31:21Z

PR Code Suggestions ✨

Category	Suggestion	Score
Possible issue	Remove unnecessary parameter from API call to prevent potential errors Remove the `repo` parameter from the `github.rest.users.block()` call as it's not required for this API endpoint and may cause an error. .github/workflows/delete-comments.yml [41-45] await github.rest.users.block({ owner: context.repo.owner, - repo: context.repo.repo, user_id: userId }); Apply this suggestion Suggestion importance[1-10]: 9 Why: Removing the `repo` parameter from the `github.rest.users.block()` call is crucial as it is not required and could cause errors, thus improving the robustness of the code.	9
Best practice	✅ Use the preferred parameter for user identification in the block operation Suggestion Impact: The commit changed the parameter from user_id to username in the block operation, aligning with the suggestion to use the preferred parameter for user identification. code diff: - const userId = context.payload.comment.user.id; - await github.rest.users.block({ - owner: context.repo.owner, - repo: context.repo.repo, - user_id: userId + const username = context.payload.comment.user.login + await github.rest.orgs.blockUser({ + org: context.repo.owner, + username: username Consider using the `username` parameter instead of `user_id` when blocking a user. The GitHub API prefers `username` for user identification in the block operation. .github/workflows/delete-comments.yml [41-45] await github.rest.users.block({ owner: context.repo.owner, - repo: context.repo.repo, - user_id: userId + username: context.payload.comment.user.login }); Apply this suggestion Suggestion importance[1-10]: 8 Why: The suggestion to use `username` instead of `user_id` aligns with GitHub API best practices for user identification, which can improve code reliability and maintainability.	8

This is too generic a word and frequently matches against comments that don't need to be deleted, nor should the user be blocked as the current workflow will do. Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>

The existing code was calling the individual "block a user" REST endpoint with incorrect parameters and never would have worked. Update it to (presumably achieve the desired outcome) block the user from the owning organisation instead. Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>

diemol

Thank you, @dnwe!

* chore(ci): correct github-script API calls Since V5 of github-script the Octokit context available via `github` no longer has REST methods directly on it, they were moved to `github.rest.*` instead. Update the references in delete-comments.yml job to match. Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com> * fix(ci): remove 'Download' from delete-comments This is too generic a word and frequently matches against comments that don't need to be deleted, nor should the user be blocked as the current workflow will do. Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com> * fix(ci): correct block user task The existing code was calling the individual "block a user" REST endpoint with incorrect parameters and never would have worked. Update it to (presumably achieve the desired outcome) block the user from the owning organisation instead. Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com> --------- Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>

dnwe force-pushed the fix-delete-comments branch from 5e3f8a0 to d412c2a Compare August 27, 2024 12:30

qodo-code-review bot added P-enhancement PR with a new feature P-bug fix PR addresses a known issue labels Aug 27, 2024

qodo-code-review bot added the Review effort [1-5]: 1 label Aug 27, 2024

dnwe added 2 commits August 27, 2024 13:58

fix(ci): remove 'Download' from delete-comments

2ec4f10

This is too generic a word and frequently matches against comments that don't need to be deleted, nor should the user be blocked as the current workflow will do. Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>

diemol approved these changes Aug 28, 2024

View reviewed changes

diemol merged commit cdc57b2 into SeleniumHQ:trunk Aug 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore(ci): correct github-script API calls#14442

chore(ci): correct github-script API calls#14442
diemol merged 3 commits intoSeleniumHQ:trunkfrom
dnwe:fix-delete-comments

dnwe commented Aug 27, 2024 •

edited by qodo-code-review bot

Loading

Uh oh!

CLAassistant commented Aug 27, 2024 •

edited

Loading

Uh oh!

qodo-code-review bot commented Aug 27, 2024

Uh oh!

qodo-code-review bot commented Aug 27, 2024 •

edited

Loading

Uh oh!

diemol left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

dnwe commented Aug 27, 2024 • edited by qodo-code-review bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

Description

Motivation and Context

Types of changes

Checklist

PR Type

Description

Changes walkthrough 📝

Uh oh!

CLAassistant commented Aug 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qodo-code-review bot commented Aug 27, 2024

PR Reviewer Guide 🔍

Uh oh!

qodo-code-review bot commented Aug 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Uh oh!

diemol left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dnwe commented Aug 27, 2024 •

edited by qodo-code-review bot

Loading

CLAassistant commented Aug 27, 2024 •

edited

Loading

qodo-code-review bot commented Aug 27, 2024 •

edited

Loading