Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unique pr comment #512

Merged
merged 12 commits into from
Jun 5, 2024
Merged

Unique pr comment #512

merged 12 commits into from
Jun 5, 2024

Conversation

pelikhan
Copy link
Member

@pelikhan pelikhan commented Jun 5, 2024

CLI Changes

  • πŸ”„ The --pull-request-comment and --pull-request-description options now create comments with a unique id, defaulting to the script id.
  • πŸ—‘οΈ The --no-pull-request-reviews-cache option has been removed.
  • πŸ› οΈ The internal logic for creating pull request reviews and updating pull request descriptions has been refactored.

Library Changes

  • πŸ› οΈ Refactored the githubUpdatePullRequestDescription function to include unique comment tags and improved the logic for updating the pull request description.
  • 🧹 Removed the cache-related code for pull request reviews, simplifying the githubCreatePullRequestReviews function.
  • πŸš€ Enhanced the githubCreateIssueComment function to sort comments by the last updated and to avoid duplication when creating new comments.
  • 🧹 Cleaned up unused imports and variables related to caching in the GitHub utility functions.

generated by genaiscript pr-describe

Copy link

github-actions bot commented Jun 5, 2024

Concerns:

  • The removal of the pullRequestReviewsCache option and related cache logic in run.ts and github.ts might impact performance if the system was relying on the cache to avoid redundant API calls. 🚦
  • The renaming of githubUpsetPullRequest to githubUpdatePullRequestDescription without a corresponding change in the function's logic might not reflect the actual functionality if the original name was intended (upsert vs update). πŸ€”
  • The change in the githubCreateIssueComment function to append the tag might result in the tag being added multiple times if the function is called repeatedly. πŸ”„

Suggested code fixes and improvements:

  • If the cache was indeed beneficial, consider reintroducing the caching mechanism to avoid unnecessary API calls. πŸ”„
  • Ensure that the renaming of the function accurately reflects its behavior (upsert implies insert or update if exists, whereas update implies modifying an existing record). If upsert behavior is desired, the logic should be updated accordingly. πŸ› οΈ
  • In githubCreateIssueComment, implement a check to ensure the tag is only appended if it does not already exist in the comment body. βœ…
// In githubCreateIssueComment, before appending the tag:
if (!body.includes(tag)) {
    body = `${body}\n\n${tag}\n\n`;
}

If these concerns are addressed, then it would be "LGTM πŸš€".

generated by genaiscript pr-review

@pelikhan pelikhan merged commit 38fae30 into main Jun 5, 2024
10 checks passed
@pelikhan pelikhan deleted the unique-pr-comment branch June 5, 2024 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant