Skip to content

Add preview links comment to PRs #1416

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add preview links comment to PRs #1416

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b626770
Add preview links comment to PRs
reakaleek Jun 23, 2025
6c50add
Update .github/workflows/preview-build.yml
reakaleek Jun 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 71 additions & 2 deletions .github/workflows/preview-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name: preview-build

on:
pull_request_target:
pull_request:
Copy link
Member

Choose a reason for hiding this comment

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

IIUC, this will not let you use write permissions scopes. Is that the expected behaviour?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is just the trigger of this worklfow in this repository.

I changed it to pull_request to be able to validate the changes on this PR.

But I also thought, I'll just keep it. As we seldom have fork contributions here.

the pull_request_target trigger is still valid in consumer repos.

types:
- opened
- synchronize
Expand Down Expand Up @@ -43,12 +43,17 @@ on:
type: string
default: 'false'
required: false
disable-comments:
description: 'Disable comments'
type: boolean
default: false
required: false

permissions:
id-token: write
deployments: write
contents: read
pull-requests: read
pull-requests: write

jobs:
match:
Expand Down Expand Up @@ -231,6 +236,70 @@ jobs:
aws cloudfront create-invalidation \
--distribution-id EKT7LT5PM8RKS \
--paths "${PATH_PREFIX}" "${PATH_PREFIX}/*"

- name: Comment on PR
continue-on-error: true
if: inputs.disable-comments != 'true' && env.MATCH == 'true' && steps.deployment.outputs.result && steps.check-files.outputs.all_changed_files
uses: actions/github-script@v7
env:
ALL_CHANGED_FILES: ${{ steps.check-files.outputs.all_changed_files }}
with:
script: |
const title = '## 🔍 Preview links for changed docs'
const changedMdFiles = process.env.ALL_CHANGED_FILES
.split(/\s+/)
.filter(i => i.endsWith('.md'))
.filter(i => !i.includes('/_snippets/'));

if (changedMdFiles.length === 0) {
return;
}

const toLink = (file) => {
const path = file
.replace('docs/', '')
.replace('/index.md', '')
.replace('.md', '');
return `[${file}](https://docs-v3-preview.elastic.dev${process.env.PATH_PREFIX}/${path})`;
}

const links = changedMdFiles.map(toLink)

const body = [
title,
...links.slice(0, 20).map(i => `- ${i}`),
]

if (links.length > 20) {
body.push('');
body.push(`<sub>There are ${links.length - 20} more links...</sub>`);
Copy link
Contributor

Choose a reason for hiding this comment

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

do these additional links get exposed anywhere?

Copy link
Member

Choose a reason for hiding this comment

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

Do you want them to be @shainaraskas ?

I guess we could hide the remainder in a <summary> in the comment?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be nice to avoid having to track down a whole pile for very complex PRs - 20 is a little on the low side. a <summary> tag would be perfect.

wonder if there's a risk of this running away into the 1000s of links in case of some low-level find/replace or a bad/old branch, so we might still want to limit it to avoid perf issues (I'll let you decide whether this is necessary)

Copy link
Member Author

Choose a reason for hiding this comment

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

My thinking was that more than 20 links are more overwhelming than helpful.

The majority of the current open PRs have one to five links. There are two outliers with 15+ links.

I'm happy to increase the number, but as you said, showing 1000s of links won't be useful.

Also, we need to consider the max pr comment character limit.

Copy link
Contributor

@leemthompo leemthompo Jun 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think there's a happy medium between 20 and 1000s 😉?

Could say

  • if less than 10 just use standard comments as we have today
  • if more than 10, whack em in a collapsible
  • cap at something like 75-100 to handle edge cases without going crazy

}

const owner = context.repo.owner;
const repo = context.repo.repo;
const issue_number = context.payload.pull_request.number;

// Post or update a single bot comment
const { data: comments } = await github.rest.issues.listComments({
owner, repo, issue_number
});
const existing = comments.find(c =>
c.user.type === 'Bot' &&
c.body.startsWith(title)
);
if (existing) {
await github.rest.issues.updateComment({
owner, repo,
comment_id: existing.id,
body: body.join('\n'),
});
} else {
await github.rest.issues.createComment({
owner, repo,
issue_number,
body:body.join('\n'),
});
}

- name: Update Link Index
if: |
Expand Down
Loading