Skip to content

Commit 96860a7

Browse files
authored
Resolve TODO comments (batch 1) (#58553)
1 parent 4936053 commit 96860a7

File tree

15 files changed

+296
-59
lines changed

15 files changed

+296
-59
lines changed

src/content-linter/lib/linting-rules/liquid-ifversion-versions.ts

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,9 @@ import {
1515
isAllVersions,
1616
getFeatureVersionsObject,
1717
isInAllGhes,
18+
isGhesReleaseDeprecated,
1819
} from '@/ghes-releases/scripts/version-utils'
19-
import { deprecated, oldestSupported } from '@/versions/lib/enterprise-server-releases'
20+
import { oldestSupported } from '@/versions/lib/enterprise-server-releases'
2021
import type { RuleParams, RuleErrorCallback } from '@/content-linter/types'
2122

2223
export const liquidIfversionVersions = {
@@ -337,19 +338,9 @@ function updateConditionals(condTagItems: any[]) {
337338
}
338339
// Checks for features that are only available in no
339340
// supported GHES releases
340-
// TODO use isGhesReleaseDeprecated
341-
if (item.versionsObjAll.ghes.startsWith('<=')) {
342-
const releaseNumber = item.versionsObjAll.ghes.replace('<=', '').trim()
343-
if (deprecated.includes(releaseNumber)) {
344-
item.action.type = 'delete'
345-
continue
346-
}
347-
} else if (item.versionsObjAll.ghes.startsWith('<')) {
348-
const releaseNumber = item.versionsObjAll.ghes.replace('<', '').trim()
349-
if (deprecated.includes(releaseNumber) || releaseNumber === oldestSupported) {
350-
item.action.type = 'delete'
351-
continue
352-
}
341+
if (isGhesReleaseDeprecated(oldestSupported, item.versionsObjAll.ghes)) {
342+
item.action.type = 'delete'
343+
continue
353344
}
354345
}
355346
if (item.versionsObj?.feature || item.fileVersionsFm?.feature) break

src/content-linter/tests/category-pages.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ describe.skip('category pages', () => {
5353
// Get links included in product index page.
5454
// Each link corresponds to a product subdirectory (category).
5555
// Example: "getting-started-with-github"
56-
const contents = fs.readFileSync(productIndex, 'utf8') // TODO move to async
56+
const contents = fs.readFileSync(productIndex, 'utf8')
5757
const data = getFrontmatterData(contents)
5858

5959
const productDir = path.dirname(productIndex)
@@ -62,7 +62,6 @@ describe.skip('category pages', () => {
6262
const categoryLinks = children
6363
// Only include category directories, not standalone category files like content/actions/quickstart.md
6464
.filter((link) => fs.existsSync(getPath(productDir, link, 'index')))
65-
// TODO this should move to async, but you can't asynchronously define tests with vitest...
6665

6766
// Map those to the Markdown file paths that represent that category page index
6867
const categoryPaths = categoryLinks.map((link) => getPath(productDir, link, 'index'))

src/early-access/scripts/what-docs-early-access-branch.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import { getOctokit } from '@actions/github'
22
import { setOutput } from '@actions/core'
33

44
async function main(): Promise<void> {
5-
// TODO Is there a lib function for this?
65
const { BRANCH_NAME, GITHUB_TOKEN } = process.env
76
if (!BRANCH_NAME) throw new Error("'BRANCH_NAME' env var not set")
87
if (!GITHUB_TOKEN) throw new Error("'GITHUB_TOKEN' env var not set")

src/frame/tests/page.ts

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -439,19 +439,6 @@ describe('catches errors thrown in Page class', () => {
439439
await expect(getPage).rejects.toThrowError('versions')
440440
})
441441

442-
// TODO - UNSKIP WHEN GHAE IS UPDATED WITH SEMVER VERSIONING
443-
test.skip('invalid versions frontmatter', async () => {
444-
async function getPage() {
445-
return await Page.init({
446-
relativePath: 'page-with-invalid-product-version.md',
447-
basePath: path.join(__dirname, '../../../src/fixtures/fixtures'),
448-
languageCode: 'en',
449-
})
450-
}
451-
452-
await expect(getPage).rejects.toThrowError('versions')
453-
})
454-
455442
test('English page with a version in frontmatter that its parent product is not available in', async () => {
456443
async function getPage() {
457444
return await Page.init({

src/frame/tests/site-tree.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { loadSiteTree } from '@/frame/lib/page-data'
77
import nonEnterpriseDefaultVersion from '@/versions/lib/non-enterprise-default-version'
88
import { formatAjvErrors } from '@/tests/helpers/schemas'
99
import type { SiteTree, Tree } from '@/types'
10+
import findPageInSiteTree from '@/frame/lib/find-page-in-site-tree'
1011

1112
const latestEnterpriseRelease = EnterpriseServerReleases.latest
1213

@@ -37,15 +38,14 @@ describe('siteTree', () => {
3738
const ghesSiteTree = siteTree.en[ghesLatest]
3839

3940
// Find a page in the tree that we know contains Liquid
40-
// TODO: use new findPageInSiteTree helper when it's available
41-
const pageWithDynamicTitle = ghesSiteTree.childPages
42-
.find((child) => child.href === `/en/${ghesLatest}/admin`)
43-
?.childPages.find(
44-
(child) => child.href === `/en/${ghesLatest}/admin/installing-your-enterprise-server`,
45-
)
41+
const pageWithDynamicTitle = findPageInSiteTree(
42+
ghesSiteTree,
43+
siteTree.en[nonEnterpriseDefaultVersion],
44+
`/en/${ghesLatest}/admin/installing-your-enterprise-server`,
45+
)
4646

4747
// Confirm the raw title contains Liquid
48-
expect(pageWithDynamicTitle?.page.title).toEqual(
48+
expect(pageWithDynamicTitle.page.title).toEqual(
4949
'Installing {% data variables.product.prodname_enterprise %}',
5050
)
5151
})

src/landings/components/SidebarProduct.tsx

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,6 @@ function RestNavListItem({ category }: { category: ProductTreeNode }) {
146146
},
147147
{ rootMargin: '0px 0px -85% 0px' },
148148
)
149-
// TODO: When we add the ## About the {title} API to each operation
150-
// we can remove the h2 here
151149
const headingsList = Array.from(document.querySelectorAll('h2, h3'))
152150

153151
for (const heading of headingsList) {

src/learning-track/middleware/learning-track.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@ export default async function learningTrack(
2727
const trackName = req.query.learn as string
2828

2929
let trackProduct = req.context.currentProduct as string
30-
// TODO: Once getDeepDataByLanguage is ported to TS
31-
// a more appropriate API would be to use `getDeepDataByLanguage<LearningTracks)(...)`
3230
const allLearningTracks = getDeepDataByLanguage(
3331
'learning-tracks',
3432
req.language!,

src/learning-track/tests/lint-data.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@ const pages = await loadPageMap(pageList)
1111
const redirects = await loadRedirects(pageList)
1212

1313
describe('learning tracks', () => {
14-
// TODO: Once getDeepDataByLanguage is ported to TS
15-
// a more appropriate API would be to use `getDeepDataByLanguage<LearningTracks)(...)`
1614
const allLearningTracks = getDeepDataByLanguage('learning-tracks', 'en') as LearningTracks
1715
const topLevels = Object.keys(allLearningTracks)
1816

src/observability/tests/logger.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -327,10 +327,6 @@ describe('createLogger', () => {
327327
logger = createLogger('file:///path/to/test.js')
328328
})
329329

330-
it('should include logger context in production logs', () => {
331-
// TODO
332-
})
333-
334330
it('should handle missing logger context gracefully in development', () => {
335331
logger.info('No context test')
336332

src/rest/scripts/update-files.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,7 @@ async function main() {
114114
// so that we don't spend time generating data files for them.
115115
if (sourceRepos.includes(REST_API_DESCRIPTION_ROOT)) {
116116
const derefDir = await readdir(TEMP_OPENAPI_DIR)
117-
// TODO: After migrating all-version.ts to TypeScript, we can remove the type assertion
118-
const currentOpenApiVersions = Object.values(allVersions).map(
119-
(elem) => (elem as any).openApiVersionName,
120-
)
117+
const currentOpenApiVersions = Object.values(allVersions).map((elem) => elem.openApiVersionName)
121118

122119
for (const schema of derefDir) {
123120
// if the schema does not start with a current version name, delete it

0 commit comments

Comments
 (0)