Skip or delete PR diff comments when empty

[colinrotherham]

This PR addresses some of the review comments in #4511 (review)

Skipping empty GitHub diff comments

We no longer post "No diff changes found." comments, except for:

1. JavaScript changes to GitHub release
2. Stylesheets changes to GitHub release

This reduces comment noise for day-to-day PRs but keeps the important two for GitHub releases

Deleting outdated GitHub diff comments

What if a PR had a diff and a comment was added

But another push results in no diff, so we'd be left with an outdated comment that was already posted

With the comment now out of date, we now remove it via GitHub REST API issues.deleteComment

[github-actions]

📋 Stats

File sizes

File	Size
dist/govuk-frontend-development.min.css	113.65 KiB
dist/govuk-frontend-development.min.js	38.28 KiB
packages/govuk-frontend/dist/govuk/all.bundle.js	77.78 KiB
packages/govuk-frontend/dist/govuk/all.bundle.mjs	73.09 KiB
packages/govuk-frontend/dist/govuk/all.mjs	3.86 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend-component.mjs	359 B
packages/govuk-frontend/dist/govuk/govuk-frontend.min.css	113.64 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend.min.js	38.27 KiB
packages/govuk-frontend/dist/govuk/i18n.mjs	5.38 KiB

Modules

File	Size
all.mjs	69.41 KiB
components/accordion/accordion.mjs	21.67 KiB
components/button/button.mjs	4.7 KiB
components/character-count/character-count.mjs	21.24 KiB
components/checkboxes/checkboxes.mjs	5.83 KiB
components/error-summary/error-summary.mjs	6.09 KiB
components/exit-this-page/exit-this-page.mjs	16.08 KiB
components/header/header.mjs	3.9 KiB
components/notification-banner/notification-banner.mjs	4.54 KiB
components/radios/radios.mjs	4.83 KiB
components/skip-link/skip-link.mjs	3.81 KiB
components/tabs/tabs.mjs	9.66 KiB

View stats and visualisations on the review app

---

Action run for aabe22d

[romaricpascal]

Cheers for digging into solutions for the comments on that PR 😊

I think there's a mix of things in this PR and we may want to split it into two:

1. Adding comments in a consistent order sounds fairly uncontroversial and could go on its own
2. Skipping or deleting when empty could use a bigger discussion, as we may still want a "no diff to output" comment that would reassure us that the diff did actually run (and that there wasn't a bug despite the workflow completing OK, for example).

In terms of code, the steps taken look generally fine. I'd be keen to keep the <!-- and --> wrapping the marker text inside the functions creating the comments (and now searching/deleting it), as it provides some safety that the marker won't be visible in the comment once posted.

[colinrotherham]

@romaricpascal Thanks

• Diff comment order moved to Add PR diff comments in order #4523
• Marker text <!-- and --> abstraction is hidden away again (pushed)

…Skipping or deleting when empty could use a bigger discussion, as we may still want a "no diff to output" comment that would reassure us that the diff did actually run (and that there wasn't a bug despite the workflow completing OK, for example).

☝️ Sounds good, let me know, I was happy with the noise at first but now they're everywhere 😱

I've only set skipEmpty: true on the ones I'd skip in 99eb358

[colinrotherham]

We discussed this at dev catch up so I'll add a summary 🙌

Essentially it felt a shame to delete "No diff changes found" PR comments but sadly the GitHub REST API for issues doesn't support the hide comment feature. It's only found in the GitHub GraphQL API as “minimising as OUTDATED”

Since we only added npm package diffs last week, we won't miss seeing empty comments

So to move forward we'll determine which PR diff comments can be skipped (or deleted) when empty

I've suggested we merge this PR which implements the following:

1. Published npm package diffs can be skipped or deleted
2. GitHub release diffs for CSS and JavaScript cannot be skipped or deleted

Then in future, for any skipped comments, can add a single combined summary in another PR

[romaricpascal]

Let's reduce the noise for now, and think about how to get some certainty that things have ran OK in a future PR. I'll create an issue to track that.

Thanks for updating our set up 😊 ⛵