Skip to content

Commit

Permalink
Support duplicate action comments on PRs (#52)
Browse files Browse the repository at this point in the history
The action may be run multiple times as requested. This isn't supported
by the PR comment status update as it presumes a single run of a v2
style workspace. To support this case we now dedupe comments on workflow
and job name. The `view` link is moved to a `<workflow> / <job>
(pull_request)` link in the comment description to help users
differentiate runs. The name matches the status name below the CI fold.
Note: there is no step level ID provided to the action so each
invocation must be unique combination of workflow and job name otherwise
the comment will update a previous comment.

<img width="738" alt="Screenshot 2024-07-18 at 9 40 35 PM"
src="https://github.com/user-attachments/assets/5f62342d-59f3-4be6-a6e6-eb139d10a2f3">

Fixes #51
  • Loading branch information
emcfarlane authored Jul 22, 2024
1 parent 1bf85d6 commit 3f33586
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 19 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ jobs:
"❌ failed (1)",
"⏩ skipped",
];
const commentTag = "<!-- Buf results -->";
const commentTag = "<!-- buf Buf action CI:test-comment -->";
const { owner, repo } = context.repo;
const prNumber = context.payload.pull_request?.number;
if (!prNumber) {
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ Add these parameters under the `with` section of the `uses` step in the workflow
| `paths` | Limit to specific files or directories (separated by newlines). | |
| `exclude_imports` | Exclude files imported by the target modules. | False |
| `exclude_paths` | Exclude specific files or directories, e.g. "proto/a/a.proto", "proto/a" (separated by newlines). | |
| `pr_comment` | Comment the results on the pull request. | Only on pull requests |
| `pr_comment` | Comment the results on the pull request. The workflow and job name combination must be unique. | Only on pull requests |
| `format` | Whether to run the formatting step. | Runs on pushes to Git PR |
| `lint` | Whether to run the linting step. | Runs on pushes to Git PR |
| `breaking` | Whether to run the breaking change detection step. | Runs on pushes to Git PR |
Expand Down
2 changes: 1 addition & 1 deletion action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ inputs:
default: "false"
pr_comment:
description: |-
Comment on the pull request with the results of each step.
Comment on the pull request with the results of each step. The workflow and job name combination must be unique.
required: false
default: ${{ github.event_name == 'pull_request' }}

Expand Down
17 changes: 9 additions & 8 deletions dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -45607,9 +45607,12 @@ async function downloadBuf(version) {
// See the License for the specific language governing permissions and
// limitations under the License.

// commentTag is the tag used to identify the comment. This is a non-visible
// string injected into the comment body.
const commentTag = "<!-- Buf results -->";

// commentTag returns the tag used to identify the comment. This is a non-visible
// string injected into the comment body. It is unique to the workflow and job.
function commentTag() {
return `<!-- buf ${lib_github.context.workflow}:${lib_github.context.job} -->`;
}
// findCommentOnPR finds the comment on the PR that contains the Buf results.
// If the comment is found, it returns the comment ID. If the comment is not
// found, it returns undefined.
Expand All @@ -45625,7 +45628,7 @@ async function findCommentOnPR(context, github) {
repo: repo,
issue_number: prNumber,
});
const previousComment = comments.find((comment) => comment.body?.includes(commentTag));
const previousComment = comments.find((comment) => comment.body?.includes(commentTag()));
if (previousComment) {
core.info(`Found previous comment ${previousComment.id}`);
return previousComment.id;
Expand All @@ -45645,7 +45648,7 @@ async function commentOnPR(context, github, commentID, body) {
const content = {
owner: owner,
repo: repo,
body: body + commentTag,
body: body + commentTag(),
};
if (commentID) {
await github.rest.issues.updateComment({
Expand Down Expand Up @@ -45758,7 +45761,7 @@ async function main() {
// Comment on the PR with the summary, if requested.
if (inputs.pr_comment) {
const commentID = await findCommentOnPR(lib_github.context, github);
await commentOnPR(lib_github.context, github, commentID, `The latest Buf updates on your PR.\n\n${summary.stringify()}`);
await commentOnPR(lib_github.context, github, commentID, `The latest Buf updates on your PR. Results from workflow <a href="${lib_github.context.serverUrl}/${lib_github.context.repo.owner}/${lib_github.context.repo.repo}/actions/runs/${lib_github.context.runId}">${lib_github.context.workflow} / ${lib_github.context.job} (pull_request)</a>.\n\n${summary.stringify()}`);
}
// Write the summary to a file defined by GITHUB_STEP_SUMMARY.
// NB: Write empties the buffer and must be after the comment.
Expand All @@ -45782,15 +45785,13 @@ function createSummary(inputs, steps, moduleNames) {
{ data: "Format", header: true },
{ data: "Lint", header: true },
{ data: "Breaking", header: true },
{ data: "Run", header: true },
{ data: "Updated (UTC)", header: true },
],
[
message(steps.build),
message(steps.format),
message(steps.lint),
message(steps.breaking),
`<a href="${lib_github.context.serverUrl}/${lib_github.context.repo.owner}/${lib_github.context.repo.repo}/actions/runs/${lib_github.context.runId}">view</a>`,
new Date().toLocaleString("en-US", {
day: "numeric",
month: "short",
Expand Down
13 changes: 8 additions & 5 deletions src/comment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,15 @@
// limitations under the License.

import * as core from "@actions/core";
import { context } from "@actions/github";
import { Context } from "@actions/github/lib/context";
import { GitHub } from "@actions/github/lib/utils";

// commentTag is the tag used to identify the comment. This is a non-visible
// string injected into the comment body.
const commentTag = "<!-- Buf results -->";
// commentTag returns the tag used to identify the comment. This is a non-visible
// string injected into the comment body. It is unique to the workflow and job.
function commentTag(): string {
return `<!-- buf ${context.workflow}:${context.job} -->`;
}

// findCommentOnPR finds the comment on the PR that contains the Buf results.
// If the comment is found, it returns the comment ID. If the comment is not
Expand All @@ -39,7 +42,7 @@ export async function findCommentOnPR(
issue_number: prNumber,
});
const previousComment = comments.find((comment) =>
comment.body?.includes(commentTag),
comment.body?.includes(commentTag()),
);
if (previousComment) {
core.info(`Found previous comment ${previousComment.id}`);
Expand All @@ -66,7 +69,7 @@ export async function commentOnPR(
const content = {
owner: owner,
repo: repo,
body: body + commentTag,
body: body + commentTag(),
};
if (commentID) {
await github.rest.issues.updateComment({
Expand Down
4 changes: 1 addition & 3 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ async function main() {
context,
github,
commentID,
`The latest Buf updates on your PR.\n\n${summary.stringify()}`,
`The latest Buf updates on your PR. Results from workflow <a href="${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}">${context.workflow} / ${context.job} (pull_request)</a>.\n\n${summary.stringify()}`,
);
}
// Write the summary to a file defined by GITHUB_STEP_SUMMARY.
Expand Down Expand Up @@ -101,15 +101,13 @@ function createSummary(
{ data: "Format", header: true },
{ data: "Lint", header: true },
{ data: "Breaking", header: true },
{ data: "Run", header: true },
{ data: "Updated (UTC)", header: true },
],
[
message(steps.build),
message(steps.format),
message(steps.lint),
message(steps.breaking),
`<a href="${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}">view</a>`,
new Date().toLocaleString("en-US", {
day: "numeric",
month: "short",
Expand Down

0 comments on commit 3f33586

Please sign in to comment.