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

GitHub - Feature delete outdated comments #467

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

datpmt
Copy link

@datpmt datpmt commented Dec 12, 2024

  • Remove outdated comments that were previously left on the code if the user has pushed new commits addressing those comments.
  • Ensure that all resolved comments are cleared to keep the review process clean and up-to-date.

@datpmt datpmt requested a review from a team as a code owner December 12, 2024 09:25
Copy link
Member

@ashkulz ashkulz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late feedback, I'm not sure this is a good idea as-is.

  • A lot of workflows would prefer that the PR comment is marked as resolved, rather than be deleted altogether. It might be acceptable if this is disabled by default, but you can opt-in via some configuration option.
  • Sometimes there is conversation on a PR comment from pronto and I'm not sure what the API does -- does it just delete the thread or just the comment? Either way, it's not going to look good even if the thread is retained -- would like screenshots of how both scenarios look like.
  • It's implemented just for GitHub PRs, but I'm not sure why it couldn't extent to Gitlab MRs which also have the same logic.

@ashkulz
Copy link
Member

ashkulz commented Jan 11, 2025

See this code for something which should work for resolving the comments, since there's no direct API for this.

@ragingdave
Copy link

I would say the default should be resolving rather than deleting if possible with a setting to delete as an option. If there was back on forth on a pronto comment thread and it just vanished due to updating a style rule/changing code that artifact of discussion would be lost.

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.

3 participants