Skip to content

Commit 8e658e7

Browse files
heiskrCopilot
heiskr
and
Copilot
authored
Tighten code comments in src/workflows scripts (#63435)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7c52b57f-2c29-4aa1-8233-99d0d6e13571
1 parent 986195e commit 8e658e7

28 files changed

Lines changed: 182 additions & 438 deletions

‎src/workflows/action-context.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { readFileSync } from 'fs'
22

3-
// Parses the action event payload sets repo and owner to an object from runner environment
43
export function getActionContext() {
54
if (!process.env.GITHUB_EVENT_PATH) {
65
if (!process.env.CI) {

‎src/workflows/benchmark-pages.ts‎

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,13 @@
1-
/**
2-
* Benchmarks page load times across the local docs server.
3-
*
4-
* Hits every page via the article API and/or HTML routes, across
5-
* configurable languages and versions. Reports errors and slow pages.
6-
*
7-
* Assumes the server is already running on --port (default 4000).
8-
*
9-
* Usage:
10-
* npx tsx src/workflows/benchmark-pages.ts [options]
11-
*
12-
* Options:
13-
* --port <number> Server port (default: 4000)
14-
* --langs <codes> Comma-separated language codes, or "all" (default: en)
15-
* --versions <slugs> Comma-separated version slugs, or "all" (default: free-pro-team@latest)
16-
* --modes <modes> Comma-separated: article-body, article-meta, html, or "all" (default: article-body)
17-
* --sample <number> Random sample size per lang/version (default: all pages)
18-
* --slow <ms> Threshold in ms to flag as slow (default: 500)
19-
* --concurrency <n> Max concurrent requests (default: 1)
20-
* --json <path> Write JSON results to file (for CI consumption)
21-
*/
1+
// Benchmarks latency against an already-running local docs server.
2+
//
3+
// Usage:
4+
// npx tsx src/workflows/benchmark-pages.ts [options]
5+
//
6+
// Defaults: port 4000, language en, version free-pro-team@latest, mode
7+
// article-body, slow threshold 500 ms, concurrency 1.
8+
// Modes: article-body, article-meta, html, or all.
9+
// Use --langs and --versions for comma-separated lists or all, --sample to limit
10+
// pages per language and version, and --json to write CI-readable results.
2211

2312
import { parseArgs } from 'node:util'
2413

‎src/workflows/check-content-type.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@ const { CHANGED_FILE_PATHS, CONTENT_TYPE } = process.env
77
main()
88

99
async function main() {
10-
// CHANGED_FILE_PATHS is a string of space-separated
11-
// file paths. For example:
12-
// 'content/path/foo.md content/path/bar.md'
10+
// CHANGED_FILE_PATHS is space-separated: content/actions/index.md content/admin/index.md
1311
const filePaths = CHANGED_FILE_PATHS?.split(' ') || []
1412
const containsRai = checkContentType(filePaths, CONTENT_TYPE || '')
1513
if (containsRai.length === 0) {

‎src/workflows/content-changes-table-comment-cli.ts‎

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,10 @@
1-
// [start-readme]
1+
// Runs the content-changes-table code locally instead of waiting on a PR run of
2+
// .github/workflows/review-comment.yml, which runs it on pull_request_target.
3+
// Requires GITHUB_TOKEN with content and pull request read access, plus APP_URL
4+
// set to the review environment or production.
25
//
3-
// For testing the GitHub Action that executes
4-
// src/workflows/content-changes-table-comment.ts but doing it
5-
// locally.
6-
// This is more convenient and faster than relying on seeing that the
7-
// Action produces in a PR. Especially since
8-
// .github/workflows/comment-content-changes-table.yml only runs
9-
// on `pull_request_target`.
10-
//
11-
// To try it you need to generate a local `GITHUB_TOKEN` that has read-access
12-
// "content" and "pull requests" on the repo.
13-
// You also need to set an APP_URL which can be the domain of the
14-
// review environment or just the production domain. Example:
15-
//
16-
//
17-
// export GITHUB_TOKEN=github_pat_11AAAG.....
18-
// export APP_URL=https://docs.github.com
19-
// tsx src/workflows/content-changes-table-comment-cli.ts github docs-internal main 4a0b0f2
20-
//
21-
// [end-readme]
6+
// Usage:
7+
// npx tsx src/workflows/content-changes-table-comment-cli.ts github docs-internal main 4a0b0f2
228

239
import { program } from 'commander'
2410
import main from '@/workflows/content-changes-table-comment'

‎src/workflows/content-changes-table-comment.ts‎

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
1-
// To test this locally, outside of Actions, run
2-
// src/workflows/content-changes-table-comment-cli.ts. Its file header has the
3-
// instructions.
1+
// Run src/workflows/content-changes-table-comment-cli.ts to test this outside Actions.
42

53
import fs from 'node:fs'
64
import path from 'node:path'
@@ -21,15 +19,12 @@ import { inLiquid } from './lib/in-liquid'
2119
const { GITHUB_TOKEN, APP_URL, BASE_SHA, HEAD_SHA } = process.env
2220
const context = github.context
2321

24-
// Max table size in characters. peter-evans/create-or-update-comment allows a
25-
// 2^16 character comment, but the table measures itself near the end of
26-
// rendering, before the key is added, so this stays at 2^15 for headroom. See
27-
// github/docs-engineering#1849 and peter-evans/create-or-update-comment#271.
22+
// peter-evans/create-or-update-comment allows 65,536-character comments. This
23+
// table measures itself before adding the key, so 32,768 leaves headroom.
2824
const MAX_COMMENT_SIZE = 32768
2925

3026
const PROD_URL = 'https://docs.github.com'
3127

32-
// When this file is invoked directly from action as opposed to being imported
3328
if (import.meta.url.endsWith(process.argv[1])) {
3429
const baseOwner = context.payload.pull_request!.base.repo.owner.login
3530
const baseRepo = context.payload.pull_request!.base.repo.name
@@ -51,7 +46,7 @@ async function main(owner: string, repo: string, baseSHA: string, headSHA: strin
5146

5247
const octokit = retryingGithub(GITHUB_TOKEN)
5348

54-
// The list of file changes, which works even for a head commit from a fork.
49+
// Compare through the base repo so forked head commits work.
5550
const response = await octokit.rest.repos.compareCommitsWithBasehead({
5651
owner,
5752
repo,
@@ -102,12 +97,11 @@ async function main(owner: string, repo: string, baseSHA: string, headSHA: strin
10297
const fileName = file.filename.slice(pathPrefix.length)
10398
const fileUrl = fileName.replace('/index.md', '').replace(/\.md$/, '')
10499

105-
// this script is called from the main branch, so we need the API call to get the contents from the branch, instead
100+
// The workflow runs from main, so request the file from the changed branch.
106101
const fileContents = await getContents(
107102
owner,
108103
repo,
109-
// `getContents()` 404s on a file that no longer exists, so for a
110-
// removed file read the base sha to get metadata about what it was.
104+
// Removed files need the base SHA because getContents 404s at the head SHA.
111105
file.status === 'removed' ? baseSHA : headSHA,
112106
file.filename,
113107
)
@@ -199,33 +193,29 @@ function makeRow({
199193
contentCell += `[\`${fileName}\`](${sourceUrl})`
200194

201195
try {
202-
// getApplicableVersions() throws on missing, invalid or unsupported
203-
// versions frontmatter. Remove the try/catch once
204-
// github/docs-engineering#1821 is fixed.
196+
// getApplicableVersions throws for missing, invalid, or unsupported versions frontmatter.
205197
const fileVersions: string[] = getApplicableVersions(data?.versions)
206198

207199
for (const plan in allVersionShortnames) {
208-
// `plan` is the short name, e.g. fpt, used as the link label.
209-
// allVersionShortnames[plan] is the plan name, e.g. free-pro-team, used
210-
// to pick the file's matching versions. Most plans link differently.
200+
// Plan shortnames, for example fpt, label links; full names match versions.
211201
const versions = fileVersions.filter((fileVersion) =>
212202
fileVersion.includes(allVersionShortnames[plan]),
213203
)
214204

215205
if (versions.length === 1) {
216206
if (versions.toString() === nonEnterpriseDefaultVersion) {
217-
// omit version from fpt url
207+
// Default free-pro-team URLs omit the version segment.
218208

219209
reviewCell += `[${plan}](${APP_URL}/${fileUrl})<br>`
220210
prodCell += `[${plan}](${PROD_URL}/${fileUrl})<br>`
221211
} else {
222-
// for non-versioned releases (ghec) use full url
212+
// Other single-version releases use the full version URL.
223213

224214
reviewCell += `[${plan}](${APP_URL}/${versions}/${fileUrl})<br>`
225215
prodCell += `[${plan}](${PROD_URL}/${versions}/${fileUrl})<br>`
226216
}
227217
} else if (versions.length) {
228-
// for ghes releases, link each version
218+
// GHES releases link each matching version.
229219

230220
reviewCell += `${plan}@ `
231221
prodCell += `${plan}@ `
@@ -246,8 +236,7 @@ function makeRow({
246236
let note = ''
247237
if (file.status === 'removed') {
248238
note = 'removed'
249-
// If the file was removed, the `reviewCell` no longer makes sense
250-
// since it was based on looking at the base sha.
239+
// Removed files do not exist in the review environment, so review links do not apply.
251240
reviewCell = 'n/a'
252241
} else if (fromReusable) {
253242
note += 'from reusable'

‎src/workflows/delete-orphan-translation-files.ts‎

Lines changed: 11 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,19 @@
1-
/**
2-
* This script will delete files from a translation repo of files that
3-
* only exist there and not "here". Here being the docs repo.
4-
* It will only look at *.md files in `content/` and
5-
* only look at *.md and *.yml files in `data/`.
6-
*
7-
* If executed with `--dry-run` it will only print what it would delete.
8-
*
9-
* To avoid deleting too many files at once, which can make PRs too big,
10-
* there's a `--max <number>` options which is defaulted to 100.
11-
*
12-
* To run this locally, check out a translation repo and then run it like this:
13-
*
14-
* git clone git@github.com:github/docs-internal.ja-jp.git /tmp/docs-internal.ja-jp
15-
* npm run delete-orphan-translation-files -- /tmp/docs-internal.ja-jp
16-
*
17-
* Note that it doesn't execute `git rm ...` for you. Just regular
18-
* file deletion. It's up to you now to commit and push.
19-
*/
20-
211
import fs from 'fs'
222
import path from 'path'
233

244
import { program } from 'commander'
255
import walkFiles from '@/workflows/walk-files'
266
import { ROOT } from '@/frame/lib/constants'
277

8+
// Deletes orphaned translation content files from a checked-out translation repo.
9+
// It deletes files from the working tree only; it does not stage removals with git rm.
10+
//
11+
// Usage:
12+
// npm run delete-orphan-translation-files -- /tmp/docs-internal.ja-jp
13+
//
14+
// Use --dry-run to print deletions without removing files. --max defaults to 100
15+
// so one run does not create an oversized PR.
16+
2817
program
2918
.description('Delete orphan translation files')
3019
.option('--dry-run', 'Just print what it would delete')
@@ -86,27 +75,9 @@ function main(root: string, options: Options) {
8675
)
8776
}
8877

78+
// Walk content only. Translated content can still use {% data variables.x %} after English
79+
// deletes data/variables/x.yml, so translated data files may be orphans on purpose.
8980
function getContentAndDataFiles(root: string) {
90-
// The reason we're only looking at content files, and not data files,
91-
// is because data files can be *included* in content files.
92-
// Best illustrated with an imaginary example:
93-
//
94-
// Suppose there exists, in English, a `content/some-page.md` and
95-
// a `data/variables/some-var.yml`.
96-
// The English content contains: `{% data variables.some-var.some-thing %}`
97-
// Soon enough, this is present in the translations too.
98-
// Then, the English writer decides to stop referencing that variable
99-
// in `content/some-page.md`. And additionally, since no content references
100-
// the file, they also decide to `git rm data/variables/some-var.yml`.
101-
// At this point, there's technically an "orphan" file in the translation
102-
// repo that doesn't have an equivalent in the English repo. But! The
103-
// translation's copy of `content/some-page.md` might still *refer*
104-
// to `{% data variables.some-var.some-thing %}` since it hasn't yet
105-
// picked up that the English content changed.
106-
//
107-
// In conclusion, we need to be OK with the data files in translations
108-
// being potentially "full of orphans" because they might still be
109-
// referred to the in the content files.
11081
return walkFiles(path.join(root, 'content'), ['.md'])
11182
}
11283

‎src/workflows/find-past-built-pr.ts‎

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,16 @@ import github from './github'
44
import { getActionContext } from './action-context'
55
import { octoSecondaryRatelimitRetry } from './secondary-ratelimit-retry'
66

7-
// Marker used to dedupe the "gone to production" comment across reruns and
8-
// across the previous workflow that posted it as github-actions[bot].
7+
// This marker dedupes production comments even when another account posted them.
98
export const GONE_TO_PRODUCTION_MARKER = '<!-- GONE_TO_PRODUCTION -->'
109

11-
// The merge queue can batch multiple PRs into a single deploy. We walk back
12-
// from the deployed HEAD commit through `main`'s (linear, squash-merged)
13-
// ancestry to find every PR in the batch. This is bounded by the deployed SHA,
14-
// so it can only ever include already-deployed PRs, never a later batch.
10+
// The merge queue can batch multiple PRs into a single deploy. Walk back from
11+
// the deployed HEAD commit through main's linear squash-merged ancestry to find
12+
// every PR in the batch. The deployed SHA bounds the search, so it cannot include
13+
// a later batch.
1514
//
16-
// Default to the merge queue's max batch size. Over-reaching into a previous,
17-
// already-notified batch is harmless: those PRs are genuinely in production and
18-
// the marker dedupe skips any that already have the comment.
15+
// Default to the merge queue's max batch size. Overshooting into an already
16+
// notified batch is harmless because marker dedupe skips existing comments.
1917
const BATCH_MAX_COMMITS = parseInt(process.env.DEPLOY_BATCH_MAX_COMMITS || '5', 10)
2018

2119
export const COMMENT_BODY = `${GONE_TO_PRODUCTION_MARKER}
@@ -27,8 +25,7 @@ If you don't see updates when expected, try adding a random query string to the
2725
If that shows the expected content, it would indicate that the CDN is "overly caching" the page still. It will eventually update, but it can take a while.
2826
`
2927

30-
// GitHub appends "(#1234)" to the title of a squash-merge commit. Grab the last
31-
// such reference on the title line, which is the merged PR number.
28+
// GitHub appends the merged PR number to squash-merge commit titles. Grab the last one.
3229
export function extractPrNumber(commitMessage: string): number | null {
3330
const title = commitMessage.split('\n')[0]
3431
const matches = [...title.matchAll(/\(#(\d+)\)/g)]
@@ -39,7 +36,6 @@ export function extractPrNumber(commitMessage: string): number | null {
3936
return parseInt(last[1], 10)
4037
}
4138

42-
// Returns the PR numbers in the deploy batch, newest first, deduplicated.
4339
export async function findBatchPrNumbers(
4440
octokit: Octokit,
4541
owner: string,
@@ -63,9 +59,8 @@ export async function findBatchPrNumbers(
6359

6460
export type CommentResult = 'created' | 'exists' | 'locked'
6561

66-
// Posts the production comment on a single PR, unless it is locked or already
67-
// has the comment (detected by the marker, regardless of which account authored
68-
// it). Idempotent: safe to call on every rerun.
62+
// Posts the production comment on unlocked PRs unless the marker already exists
63+
// under any author. Idempotent across reruns.
6964
export async function ensureProductionComment(
7065
octokit: Octokit,
7166
owner: string,

‎src/workflows/fm-utils.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { existsSync, readFileSync } from 'fs'
22
import matter from '@gr2m/gray-matter'
33

4-
// The file paths whose frontmatter `contentType` matches `contentType`.
54
export function checkContentType(filePaths: string[], contentType: string) {
65
const unallowedChangedFiles = []
76
for (const filePath of filePaths) {

‎src/workflows/fr-add-docs-reviewers-requests.ts‎

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -107,11 +107,7 @@ async function getAllOpenPRs() {
107107
async function run() {
108108
const prData = await getAllOpenPRs()
109109

110-
// Get the PRs that are:
111-
// - not draft
112-
// - not a train
113-
// - are requesting a review by docs-reviewers
114-
// - have not already been reviewed on behalf of docs-reviewers
110+
// Keep PRs that still need the requested docs-reviewers review.
115111
const prs = prData.filter(
116112
(pr) =>
117113
!pr.isDraft &&
@@ -177,10 +173,7 @@ async function run() {
177173

178174
const projectID = projectData.organization.projectV2.id
179175

180-
// Get the IDs of the last 100 items on the board.
181-
// Until we have a way to check from a PR whether the PR is in a project,
182-
// this is how we (roughly) avoid overwriting PRs that are already on the board.
183-
// If we are overwriting items, query for more items.
176+
// The last 100 board items approximate membership; query more if fields get overwritten.
184177
const existingItemIDs = projectData.organization.projectV2.items.nodes.map(
185178
(node: { id: string }) => node.id,
186179
)
@@ -200,10 +193,7 @@ async function run() {
200193

201194
const itemIDs = await addItemsToProject(prIDs, projectID)
202195

203-
// If an item already existed on the project, the existing ID will be returned.
204-
// Exclude existing items going forward.
205-
// Until we have a way to check from a PR whether the PR is in a project,
206-
// this is how we (roughly) avoid overwriting PRs that are already on the board
196+
// Existing project items reuse their IDs, so skip them before populating fields.
207197
const newItemIDs: string[] = []
208198
const newItemAuthors: string[] = []
209199
for (let index = 0; index < itemIDs.length; index++) {
@@ -219,7 +209,7 @@ async function run() {
219209
return
220210
}
221211

222-
// for...of rather than forEach because the body awaits.
212+
// Use for...of because the body awaits.
223213
for (const [index, itemID] of newItemIDs.entries()) {
224214
const updateProjectV2ItemMutation = generateUpdateProjectV2ItemFieldMutation({
225215
item: itemID,
@@ -241,7 +231,7 @@ async function run() {
241231
contributorTypeID,
242232
contributorType,
243233
sizeTypeID,
244-
sizeType: sizeMediumID, // We need to provide something here, defaulting to 'medium' or 'M'
234+
sizeType: sizeMediumID, // The board requires size, so default to M.
245235
featureID,
246236
authorID,
247237
headers: {

‎src/workflows/generate-llms-txt.ts‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,16 @@
11
// Generates an llms.txt file from popularity data and the docs page catalog.
22
//
33
// Usage:
4-
// npm run generate-llms-txt -- --config data/llms-txt/config-docs.yml --output data/llms-txt/docs.md
5-
// npm run generate-llms-txt -- --config data/llms-txt/config-monolith.yml --output /tmp/monolith.md
4+
// npm run generate-llms-txt -- \
5+
// --config data/llms-txt/config-docs.yml \
6+
// --output data/llms-txt/docs.md
7+
// npm run generate-llms-txt -- \
8+
// --config data/llms-txt/config-monolith.yml \
9+
// --output /tmp/monolith.md
610
//
7-
// Both targets layer overrides on top of data/llms-txt/config-default.yml.
8-
// Writers can edit categories, pinned pages, thresholds, and copy in the
9-
// configs without touching this script.
10-
//
11-
// Requires DOCS_BOT_PAT_BASE for fetching popularity data from
12-
// github/docs-internal-data.
11+
// Each target overrides data/llms-txt/config-default.yml, so writers can change
12+
// categories, pinned pages, thresholds, and copy without editing this script.
13+
// Requires DOCS_BOT_PAT_BASE because popularity data lives in github/docs-internal-data.
1314

1415
import fs from 'fs'
1516

0 commit comments

Comments
 (0)