Trim excessive comments in src/workflows, part 2 (#63285) · github/docs@fe25bed · GitHub
Skip to content

Commit fe25bed

Browse files
heiskrCopilot
heiskr
and
Copilot
authored
Trim excessive comments in src/workflows, part 2 (#63285)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 17588eeb-788a-4f82-9d36-b5079fb36521
1 parent 2ef3bdb commit fe25bed

12 files changed

Lines changed: 60 additions & 151 deletions

‎src/workflows/benchmark-pages.ts‎

Lines changed: 0 additions & 4 deletions

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

Lines changed: 16 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,6 @@
1-
/**
2-
* Hi there! 👋
3-
* To test this code locally, outside of Actions, you need to run
4-
* the script src/workflows/content-changes-table-comment-cli.ts
5-
*
6-
* See the instructions in the doc string comment at the
7-
* top of src/workflows/content-changes-table-comment-cli.ts
8-
*/
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.
94

105
import fs from 'node:fs'
116
import path from 'node:path'
@@ -26,11 +21,10 @@ import { inLiquid } from './lib/in-liquid'
2621
const { GITHUB_TOKEN, APP_URL, BASE_SHA, HEAD_SHA } = process.env
2722
const context = github.context
2823

29-
// the max size of the comment (in bytes)
30-
// the action we use to post the comment caps out at about 144kb
31-
// see docs-engineering#1849 and peter-evans/create-or-update-comment#271 for more info.
32-
// The max size the action allows is 2^16, but our table calculates near the end
33-
// of its rendering before we add a key, so playing it safe with 2^15.
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.
3428
const MAX_COMMENT_SIZE = 32768
3529

3630
const PROD_URL = 'https://docs.github.com'
@@ -57,8 +51,7 @@ async function main(owner: string, repo: string, baseSHA: string, headSHA: strin
5751

5852
const octokit = retryingGithub(GITHUB_TOKEN)
5953

60-
// get the list of file changes from the PR
61-
// this works even if the head commit is from a fork
54+
// The list of file changes, which works even for a head commit from a fork.
6255
const response = await octokit.rest.repos.compareCommitsWithBasehead({
6356
owner,
6457
repo,
@@ -109,15 +102,12 @@ async function main(owner: string, repo: string, baseSHA: string, headSHA: strin
109102
const fileName = file.filename.slice(pathPrefix.length)
110103
const fileUrl = fileName.replace('/index.md', '').replace(/\.md$/, '')
111104

112-
// get the file contents and decode them
113105
// this script is called from the main branch, so we need the API call to get the contents from the branch, instead
114106
const fileContents = await getContents(
115107
owner,
116108
repo,
117-
// Can't get its content if it no longer exists.
118-
// Meaning, you'd get a 404 on the `getContents()` utility function.
119-
// So, to be able to get necessary meta data about what it *was*,
120-
// if it was removed, fall back to the 'base'.
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.
121111
file.status === 'removed' ? baseSHA : headSHA,
122112
file.filename,
123113
)
@@ -209,23 +199,20 @@ function makeRow({
209199
contentCell += `[\`${fileName}\`](${sourceUrl})`
210200

211201
try {
212-
// the try/catch is needed because getApplicableVersions() returns either [] or throws an error when it can't parse the versions frontmatter
213-
// try/catch can be removed if docs-engineering#1821 is resolved
214-
// i.e. for feature based versioning, like ghec: 'issue-6337'
202+
// getApplicableVersions() throws on missing, invalid or unsupported
203+
// versions frontmatter. Remove the try/catch once
204+
// github/docs-engineering#1821 is fixed.
215205
const fileVersions: string[] = getApplicableVersions(data?.versions)
216206

217207
for (const plan in allVersionShortnames) {
218-
// plan is the shortName (i.e., fpt)
219-
// allVersionShortNames[plan] is the planName (i.e., free-pro-team)
220-
221-
// walk by the plan names since we generate links differently for most plans
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.
222211
const versions = fileVersions.filter((fileVersion) =>
223212
fileVersion.includes(allVersionShortnames[plan]),
224213
)
225214

226215
if (versions.length === 1) {
227-
// for fpt and ghec
228-
229216
if (versions.toString() === nonEnterpriseDefaultVersion) {
230217
// omit version from fpt url
231218

‎src/workflows/fm-utils.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
import { existsSync, readFileSync } from 'fs'
22
import matter from '@gr2m/gray-matter'
33

4-
// Filters out files from a list of filePaths
5-
// that have a contentType property in their frontmatter
6-
// where the contentType value matches the given contentType argument
4+
// The file paths whose frontmatter `contentType` matches `contentType`.
75
export function checkContentType(filePaths: string[], contentType: string) {
86
const unallowedChangedFiles = []
97
for (const filePath of filePaths) {

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

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,6 @@ async function getAllOpenPRs() {
105105
}
106106

107107
async function run() {
108-
// Get info about open github/github PRs
109108
const prData = await getAllOpenPRs()
110109

111110
// Get the PRs that are:
@@ -136,7 +135,6 @@ async function run() {
136135
const prAuthors = prs.map((pr) => pr.author.login)
137136
console.log(`PRs found: ${prIDs}`)
138137

139-
// Get info about the docs-content review board project
140138
const projectData = await graphql<ProjectQueryResponse>(
141139
`
142140
query ($organization: String!, $projectNumber: Int!) {
@@ -177,7 +175,6 @@ async function run() {
177175
},
178176
)
179177

180-
// Get the project ID
181178
const projectID = projectData.organization.projectV2.id
182179

183180
// Get the IDs of the last 100 items on the board.
@@ -188,7 +185,6 @@ async function run() {
188185
(node: { id: string }) => node.id,
189186
)
190187

191-
// Get the ID of the fields that we want to populate
192188
const datePostedID = findFieldID('Date posted', projectData)
193189
const reviewDueDateID = findFieldID('Review due date', projectData)
194190
const statusID = findFieldID('Status', projectData)
@@ -197,13 +193,11 @@ async function run() {
197193
const sizeTypeID = findFieldID('Size', projectData)
198194
const authorID = findFieldID('Contributor', projectData)
199195

200-
// Get the ID of the single select values that we want to set
201196
const readyForReviewID = findSingleSelectID('Ready for review', 'Status', projectData)
202197
const hubberTypeID = findSingleSelectID('Hubber or partner', 'Contributor type', projectData)
203198
const docsMemberTypeID = findSingleSelectID('Docs team', 'Contributor type', projectData)
204199
const sizeMediumID = findSingleSelectID('M', 'Size', projectData)
205200

206-
// Add the PRs to the project
207201
const itemIDs = await addItemsToProject(prIDs, projectID)
208202

209203
// If an item already existed on the project, the existing ID will be returned.
@@ -225,8 +219,7 @@ async function run() {
225219
return
226220
}
227221

228-
// Populate fields for the new project items
229-
// (Using for...of instead of forEach since the function uses await)
222+
// for...of rather than forEach because the body awaits.
230223
for (const [index, itemID] of newItemIDs.entries()) {
231224
const updateProjectV2ItemMutation = generateUpdateProjectV2ItemFieldMutation({
232225
item: itemID,

‎src/workflows/get-env-inputs.ts‎

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,4 @@
1-
/*
2-
* Validates and returns an object of expected environment variables
3-
*
4-
* @param {Array<string>} options - Array of environment variables expected
5-
*
6-
* @returns {Object} - key value of expected env variables and their values
7-
*/
1+
// Validates the named environment variables and returns them as an object.
82
export function getEnvInputs(options: string[]) {
93
return Object.fromEntries(
104
options.map((envVarName) => {
@@ -17,18 +11,9 @@ export function getEnvInputs(options: string[]) {
1711
)
1812
}
1913

20-
/*
21-
* Given an environment variable key, return `true` or `false` if the
22-
* value is recognizable.
23-
* Turn 'true' or '1' into `true`. And '', '0', or 'false' into `false`.
24-
* All other values are invalid.
25-
* Now you can't accidentally set `export FOO=falsee` which as string `'falsee'`
26-
* could have been interpreted as a truthy value.
27-
*
28-
* @param {string} key - name of the environment variable
29-
*
30-
* @returns {boolean}
31-
*/
14+
// Reads an environment variable as a boolean. 'true' and '1' are true; '', '0'
15+
// and 'false' are false. Anything else throws, so a typo like
16+
// `export FOO=falsee` can't be read as truthy.
3217
export function boolEnvVar(key: string) {
3318
const value = process.env[key] || ''
3419
if (value === '' || value === 'false' || value === '0') return false

‎src/workflows/git-utils.ts‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,6 @@ export async function getTree(owner: string, repo: string, ref: string) {
6464
tree_sha: treeSha,
6565
recursive: 'true',
6666
})
67-
// only return files that match the patterns in allowedPaths
68-
// skip actions/changes files
6967
return data.tree
7068
} catch (err) {
7169
console.log('error getting tree', owner, repo, ref)
@@ -80,7 +78,6 @@ export async function getContentsForBlob(owner: string, repo: string, sha: strin
8078
repo,
8179
file_sha: sha,
8280
})
83-
// decode blob contents
8481
return Buffer.from(data.content, 'base64').toString()
8582
}
8683

@@ -91,7 +88,6 @@ export async function getContents(owner: string, repo: string, ref: string, path
9188
if (!('content' in data) || !data.content) {
9289
return await getContentsForBlob(owner, repo, data.sha)
9390
}
94-
// decode Base64 encoded contents
9591
return Buffer.from(data.content, 'base64').toString()
9692
}
9793

@@ -103,7 +99,6 @@ export async function getContentAndData(owner: string, repo: string, ref: string
10399
'content' in data && data.content
104100
? Buffer.from(data.content, 'base64').toString()
105101
: await getContentsForBlob(owner, repo, data.sha)
106-
// decode Base64 encoded contents
107102
return { content, blobSha: data.sha }
108103
}
109104

@@ -156,7 +151,7 @@ export async function createIssueComment(
156151
}
157152
}
158153

159-
// Search for a string in a file in code and return the array of paths to files that contain string
154+
// The paths of files in the repo containing any of the given strings.
160155
export async function getPathsWithMatchingStrings(
161156
strArr: string[],
162157
org: string,
@@ -238,10 +233,7 @@ async function searchCode(
238233
}
239234
}
240235

241-
// Recursively gets the contents of a directory within a repo. Returns an
242-
// array of file contents. This function could be modified to return an array
243-
// of objects that include the path and the content of the file if needed
244-
// in the future.
236+
// Recursively gets the contents of a directory within a repo.
245237
export async function getDirectoryContents(
246238
owner: string,
247239
repo: string,
@@ -260,7 +252,6 @@ export async function getDirectoryContents(
260252
const blobContents = await getContentsForBlob(owner, repo, blob.sha)
261253
files.push({ path: blob.path, content: blobContents })
262254
} else {
263-
// decode Base64 encoded contents
264255
const decodedContent = Buffer.from(blob.content, 'base64').toString()
265256
files.push({ path: blob.path, content: decodedContent })
266257
}

‎src/workflows/issue-report.ts‎

Lines changed: 0 additions & 4 deletions

0 commit comments

Comments
 (0)