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

Mark previous PR comment as outdated #628

Closed
henriquevcosta opened this issue Dec 1, 2023 · 4 comments
Closed

Mark previous PR comment as outdated #628

henriquevcosta opened this issue Dec 1, 2023 · 4 comments
Assignees

Comments

@henriquevcosta
Copy link

I've had some instances in my repos where there was a dependency review comment highlighting a vulnerability but once I fix that issue and my build runs again (now without failing) the old comment is still there and visually polluting the conversation tab of the PR.

Would it be possible for the action to hide its previous comment in the PR, marking it as outdated, so that it's still there but collapsed by default?

@febuiles
Copy link
Contributor

febuiles commented Dec 1, 2023

@henriquevcosta updating outdated comments should work by default:

Image

Note the "edited" at the end of the comment header, it went from failing with some deps to green and updated the comment in the PR (you can tell the fixes by the CI status changes).

Do you know any reproduction steps we could follow to try to reproduce?

@blaw2422
Copy link

@henriquevcosta - i just fixed this by changing the workflow configuration to always allow comments:

-          comment-summary-in-pr: on-failure
+          comment-summary-in-pr: always

@febuiles febuiles self-assigned this Dec 28, 2023
@henriquevcosta
Copy link
Author

henriquevcosta commented Jan 2, 2024

@febuiles I'm sorry I've not commented on this for a while, I was away and I still haven't been able to allocate the time to reproduce this.

Looking at the comment from blaw2422 and your example PR I'm guessing that the update will not be performed if I have comment-summary-in-pr property set to on-failure which has been the case, but I'd argue that it should update even in that scenario - at least my understanding of that property would be "if the build failed, create or update a comment. If the build succeeded, update a previous comment if it exists and if none exists then NOOP".
Is that what's implemented? Would you agree?

@febuiles
Copy link
Contributor

febuiles commented Jan 3, 2024

@henriquevcosta The comment behavior should be more consistent indeed! There's another open issue now regarding how comments work: #647, will post a link to this issue there!

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

No branches or pull requests

3 participants