Allow hiding of PR comments

[htcfreek]

It would be great if the PR comments can be hidden/marked as resolved.

This would make PR history mire clean and easier to follow.

Affected repository: Microsoft/PowerToys

Note: I don't have the permission to delete the comments.

[jsoref]

There's a feature where you can talk to the bot and it then actually does hide comments, the problem is that to do it "cheaply", I need a reference to the comment.

Otherwise, I have to search all comments to try to find the bot's comment.

One thing I might be able to do is to store my last comment in an @actions/cache.

Lemme see if that works...

[jsoref]

It works!

Here's a PR with the code where the bot collapsed the comment: jsoref@5bcda63

here's the code to support it: jsoref@5bcda63
and the workflow magic: jsoref/examples-testing@9c77e3e

I'll add this in by the weekend and try to offer a PR to PowerToys this weekend.

[htcfreek]

How does it then works and do I need special permissions?

cc: @crutkas

[htcfreek]

Doesn't it make sense to hide old comments automatic if a new one is added? If the word is fixed the old comment is useless and if the word isn't fixed it's part of the new comment.

[jsoref]

Basically the workflow will cache a reference to the comment it makes. The next time it runs, it asks for a reference to the previous comment it made from the cache.

The feature will be described here:
https://github.com/check-spelling/check-spelling/wiki/Feature%3A-Collapse-older-check-comments

The wiki entry isn't currently updated with the explanation of how it will work.

But, no, it won't require additional permissions in order to use. It will require an upgrade to the workflow.

[htcfreek]

Basically the workflow will cache a reference to the comment it makes. The next time it runs, it asks for a reference to the previous comment it made from the cache.

This happening if no spell errors are found too?

[jsoref]

I'll make it do that. I'm aware that's a thing, it'll require changing the workflow a bit more than I have so far. That's why I have a demo and said I'd get it done over the weekend instead of saying it's done.

In theory I can make the comment job run unconditionally but have it only try to do comment work if there's at least a comment to collapse or a comment to post.

[jsoref]

Fwiw, this will be shipped with 0.0.20 (hopefully very soon, still looking into one or two things that I might want to change before shipping based on some testing of public consumers).

[jsoref]

This shipped in https://github.com/check-spelling/check-spelling/releases/tag/v0.0.20

It will require a workflow refresh. (I'll send PRs to some repositories shortly)

Sample workflows:
https://github.com/check-spelling/spell-check-this/blob/main/.github/workflows/spelling.yml
https://github.com/check-spelling/spell-check-this/blob/744c66e2140fd8acaf5388efd0db3727d010d6e9/.github/workflows/spelling.yml

[zadjii-msft]

Hey is there any way that this could also automatically collapse comments directly on commits earlier in the PR history? Case in point microsoft/terminal#13603:

• Two sets of localtest fixes for July 2022 microsoft/terminal#13603 (comment) was made on a commit before I opened the PR, but from when I first pushed the branch
• The bot filed the comment on the PR (as to be expected)
• I fixed the spelling error in a46e2a3, and the bot collapsed the comment directly on the PR, but the commit comment remains?

I admittedly know very little about the GH action API 🤷

[jsoref]

Hmm. Maybe?

---

Workaround

A workaround which would suppress the comment for on: push (but retain the checking) would be this:

-    if: (success() || failure()) && needs.spelling.outputs.followup
+    if: (success() || failure()) && needs.spelling.outputs.followup && github.event_name != 'push'

It'd mean that when you pushed to a branch you wouldn't get a comment, but you could still click the ❌ to see the log which would still identify everything it didn't like.

v0.0.20 Implementation details

Right now I'm searching all the PR comments for messages that match criteria (code). -- This is frustratingly painful. But, at least I can stop once I hit the newest comment that matches.

Collapsing commit comments

In order to do what you want, i'd have to get a list of all commits in the PR and then ask each commit about its comments. I could probably stop at the first commit w/ a matching comment. For a PR w/ lots of commits (that were developed after the PR was opened), this would mean asking about lots of them (each of which may have a bunch of comments, but no matches, since they'd be suppressed by the open PR)

GitHub Step Summaries

My tentative plan is to move (some?) reporting to use GITHUB_STEP_SUMMARY so that the report would generally be in the status instead of as a comment. -- I've been leaning towards this approach, but haven't written any implementation for it, and I need to figure out some details. A coworker and I have started playing w/ GITHUB_STEP_SUMMARY for another action, so I have a little bit of experience with it.

I think for a PR, I'd still have some of a comment in the PR, but possibly much less (certainly the talk-to-bot bits, which are working quite nicely for us). But I need to think about how much of the report I would move over and how much I'd keep. It's possible that I'd move all the words to the report and only leave summary counts.

Anyway, I really haven't had enough time to think about how/what I want to put in if I split it.

[zadjii-msft]

Honestly the workaround is probably good enough? I'll chat with the rest of the team. I dunno if we're really listening to spellbot comments on commits in the first place, so we can probably live without them 😄. Thanks for the quick response!

[jsoref]

Yeah, it really depends on the work-mode. I do a lot of work long before I create a PR, but when I do that, I'm generally using force push, and certainly I've reordered to something where I won't have a commit w/ a comment before I make my PR.

If you don't care about them at all, you could just remove the on: / push: bits from the top.