-
Notifications
You must be signed in to change notification settings - Fork 23
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.
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
Changes from 1 commit
b626770
6c50add
8724a28
73066e1
fd3d3c5
acaedb5
88c30ca
af548aa
11a8817
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
name: preview-build | ||
|
||
on: | ||
pull_request_target: | ||
pull_request: | ||
types: | ||
- opened | ||
- synchronize | ||
|
@@ -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: | ||
|
@@ -112,6 +117,7 @@ jobs: | |
id: check-files | ||
uses: tj-actions/changed-files@2f7c5bfce28377bc069a65ba478de0a74aa0ca32 # v46.0.1 | ||
with: | ||
safe_output: false # set to false because we are using an environment variable to store the output and avoid command injection. | ||
reakaleek marked this conversation as resolved.
Show resolved
Hide resolved
reakaleek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
files: ${{ inputs.path-pattern != '' && inputs.path-pattern || '**' }} | ||
|
||
- name: Checkout | ||
|
@@ -231,6 +237,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>`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do these additional links get exposed anywhere? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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) There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
} | ||
|
||
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: | | ||
|
Uh oh!
There was an error while loading. Please reload this page.