Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
2512f37
feat(api-history): basic parsing
piotrpdev Jun 9, 2024
4665853
feat(api-history): try to detect basic user error
piotrpdev Jun 9, 2024
d7c370a
feat(api-history): zod validation
piotrpdev Jun 9, 2024
4947073
feat(api-history): switch to json schema and ajv instead of zod
piotrpdev Jun 11, 2024
edb99b0
refactor(api-history): remove namespace in schema title
piotrpdev Jun 18, 2024
c4b0846
feat(api-history): optimize imports, more async
piotrpdev Jun 18, 2024
51f3859
feat(api-history): remove `removed` schema property
piotrpdev Jun 18, 2024
e6bc3df
fix(api-history): add missing param to usage
piotrpdev Jun 18, 2024
dfb7b91
feat(api-history): basic tests
piotrpdev Jun 18, 2024
d609df1
feat(api-history): cli schema option
piotrpdev Jun 19, 2024
d2ae01b
fix(api-history): delete links from fixtures
piotrpdev Jun 19, 2024
16790db
feat(api-history): use mockup schema
piotrpdev Jun 19, 2024
687d78c
style(api-history): remove old comments
piotrpdev Jun 19, 2024
43cb826
feat(api-history): add format test
piotrpdev Jun 19, 2024
20ade10
test(api-history): remove snapshot, destructure regex matches
piotrpdev Jun 19, 2024
9038174
feat(api-history): use `hast-util-from-markdown` instead of regex
dsanders11 Jun 19, 2024
bac2454
style(api-history): slightly nicer formatting
piotrpdev Jun 19, 2024
25c0f6b
feat(api-history): remove redundant `validate-with-schema` flag
piotrpdev Jul 5, 2024
bd16eb8
feat(api-history): check breaking changes header exists
piotrpdev Jul 5, 2024
1c0b98e
feat(api-history): check placement
piotrpdev Jul 6, 2024
cba5b11
feat(api-history): check strings (previously check descriptions)
piotrpdev Jul 6, 2024
edc9a69
fix(api-history): change check descriptions to check strings in help
piotrpdev Jul 6, 2024
1dd1383
style(api-history): catch-up on linting
piotrpdev Jul 6, 2024
c577ec0
feat(api-history): check pull request links
piotrpdev Jul 7, 2024
f7b4866
test(api-history): explicitly specify default flags
piotrpdev Jul 7, 2024
0175428
feat(api-history): warnings, nicer errors
piotrpdev Jul 7, 2024
0ff2a8b
test(api-history): check all numbers in each test
piotrpdev Jul 7, 2024
cd54d6a
test(api-history): large amount of api history
piotrpdev Jul 8, 2024
42ab830
Merge remote-tracking branch 'origin/main' into feat/api-history
piotrpdev Jul 8, 2024
c59af26
refactor(api-history): remove `--check-pull-request-links`
piotrpdev Jul 9, 2024
1f34a44
style(api-history): catch up on lint
piotrpdev Jul 9, 2024
22c2b38
fix(api-history): unnecessary lowercase
piotrpdev Jul 15, 2024
db34c2a
fix(api-history): match api history block more consistently
piotrpdev Jul 15, 2024
f089760
feat(api-history): skip documents without html comments
piotrpdev Jul 15, 2024
3ccddfe
fix(api-history): remove debug `console.log`
piotrpdev Jul 15, 2024
43155a3
Apply suggestions from code review
piotrpdev Jul 17, 2024
8f8c461
style(api-history): lint fix
piotrpdev Jul 17, 2024
b5d6213
style(api-history): reorder imports
piotrpdev Jul 17, 2024
59e9a67
docs(api-history): comments for script options
piotrpdev Jul 17, 2024
ee64a4c
fix(api-history): remove `fs.access()` before `fs.readFile()`
piotrpdev Jul 17, 2024
ed86d93
test(api-history): use `fs.mkdtemp()` instead of fixtures folder
piotrpdev Jul 17, 2024
bbdc778
test(api-history): use newest schema
piotrpdev Jul 17, 2024
441c8a0
test(api-history): use newest `breaking-changes.md`
piotrpdev Jul 17, 2024
7a7366b
test(api-history): rename fixture to avoid conflict with other `.spec…
piotrpdev Jul 17, 2024
84a3dbc
chore: retain .md extension
dsanders11 Jul 17, 2024
4d42ea2
chore: maintainer committed yarn.lock
dsanders11 Jul 17, 2024
4f988c9
test: ignore API history files in links spec
dsanders11 Jul 17, 2024
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
Prev Previous commit
Next Next commit
feat(api-history): check pull request links
  • Loading branch information
piotrpdev committed Jul 7, 2024
commit c577ec002be100a89988c64ca097d7a76002d5bd
166 changes: 164 additions & 2 deletions bin/lint-markdown-api-history.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ import type { fromHtml as FromHtmlFunction } from 'hast-util-from-html';
import type { fromMarkdown as FromMarkdownFunction } from 'mdast-util-from-markdown';
import { dynamicImport } from '../lib/helpers';
import Ajv, { JSONSchemaType, ValidateFunction } from 'ajv';
import AdmZip = require('adm-zip');

// TODO: Use consistent style for errors and warnings

// "<any char>: <match group>"
const possibleStringRegex = /^[ \S]+?: *?(\S[ \S]+?)$/gm;
Expand All @@ -32,7 +35,6 @@ interface ApiHistory {
changes?: ChangeSchema[];
}

// TODO: Add option for CI to use to validate the PR that triggered the CI run
interface Options {
checkPlacement: boolean;
checkPullRequestLinks: boolean;
Expand All @@ -46,6 +48,124 @@ interface HTMLWithPreviousNode extends HTML {
previousNode?: Node;
}

// If you change this, you might want to update the one in the website transformer
Comment thread
piotrpdev marked this conversation as resolved.
Outdated
export interface PrReleaseVersions {
release: string | null;
backports: Array<string>;
}

// If you change this, you might want to update the one in the website transformer
export type PrReleaseVersionsContainer = { [key: number]: PrReleaseVersions };

// If you change this, you might want to update the one in the website transformer
interface PrReleaseArtifact {
data: PrReleaseVersionsContainer;
endCursor: string;
}

function getCIPrNumber(): number | null {
if (process.env.GITHUB_REF_NAME?.endsWith('/merge')) {
// https://docs.github.com/en/actions/learn-github-actions/variables#default-environment-variables
return Number(process.env.GITHUB_REF_NAME.split('/')[0]);
} else if (process.env.CIRCLE_PULL_REQUEST?.includes('/pull/')) {
// https://circleci.com/docs/variables/#built-in-environment-variables
return Number(process.env.CIRCLE_PULL_REQUEST.split('/').at(-1));
} else {
return null;
}
}

let _allPrReleaseVersions: PrReleaseVersionsContainer;

// If you change this, you might want to update the one in the website transformer
// TODO: Change this when GH_TOKEN isn't needed to fetch PR release versions anymore
async function getAllPrReleaseVersions(): Promise<PrReleaseVersionsContainer> {
try {
if (_allPrReleaseVersions) {
return _allPrReleaseVersions;
}

if (process.env.NODE_ENV === 'test') {
Comment thread
piotrpdev marked this conversation as resolved.
Outdated
const versions: PrReleaseVersionsContainer = {
22533: {
release: '',
backports: [] as string[],
},
26789: {
release: '',
backports: [] as string[],
},
37094: {
release: '',
backports: [] as string[],
},
};

_allPrReleaseVersions = versions;

const ciPrNumber = getCIPrNumber();
if (ciPrNumber) {
console.log(`Detected CI PR number '${ciPrNumber}', adding to list of PRs.`);
_allPrReleaseVersions[ciPrNumber] = {
release: '',
backports: [],
};
}

return _allPrReleaseVersions;
}

if (!process.env.GH_TOKEN) {
throw new Error(
`Error while checking PR links: GH_TOKEN is required for fetching PR release versions.`,
);
}

const fetchOptions = {
method: 'GET',
headers: {
Accept: 'application/vnd.github+json',
'X-GitHub-Api-Version': '2022-11-28',
Authorization: `Bearer ${process.env.GH_TOKEN}`,
},
};

const artifactsListResponse = await fetch(
'https://api.github.com/repos/electron/website/actions/artifacts',
Comment thread
piotrpdev marked this conversation as resolved.
Outdated
fetchOptions,
);
const latestArtifact = (await artifactsListResponse.json()).artifacts
.filter(({ name }: { name: string }) => name === 'resolved-pr-versions')
.sort((a: { id: number }, b: { id: number }) => a.id > b.id)[0];

const archiveDownloadResponse = await fetch(latestArtifact.archive_download_url, fetchOptions);
const buffer = Buffer.from(await archiveDownloadResponse.arrayBuffer());

const zip = new AdmZip(buffer);
const parsedData = JSON.parse(zip.readAsText(zip.getEntries()[0]!)) as PrReleaseArtifact;
Comment thread
piotrpdev marked this conversation as resolved.
Outdated

if (!parsedData?.data) {
throw new Error(`No data found in the PR release versions artifact.`);
}

_allPrReleaseVersions = parsedData.data;

const ciPrNumber = getCIPrNumber();
if (ciPrNumber) {
console.log(`Detected CI PR number '${ciPrNumber}', adding to list of PRs.`);
_allPrReleaseVersions[ciPrNumber] = {
release: '',
backports: [],
};
}

return _allPrReleaseVersions;
} catch (error) {
console.error(`Error while checking PR links:\n${error}`);
process.exit(1);
}
}

export async function findPossibleApiHistoryBlocks(
content: string,
): Promise<HTMLWithPreviousNode[]> {
Expand Down Expand Up @@ -204,7 +324,7 @@ async function main(
const isLastCharNonAlphaNumeric =
trimmedMatchedGroup.at(-1)?.match(nonAlphaNumericDotRegex) !== null;
if (isFirstCharNonAlphaNumeric || isLastCharNonAlphaNumeric) {
console.error(
console.warn(
`Warning parsing ${filepath}\n
Possible string value starts/ends with a non-alphanumeric character,\n
this might cause issues when parsing the YAML (might not throw an error):\n
Expand Down Expand Up @@ -295,6 +415,39 @@ async function main(
}
}

if (checkPullRequestLinks) {
const allPrReleaseVersions = await getAllPrReleaseVersions();

const safeHistory = unsafeHistory as ApiHistory;
const prsInHistory: Array<string> = [];

// Copied from <https://github.com/electron/website/blob/a5d30f1ede6b20ea00d487c198c71560745063ab/src/transformers/api-history.ts#L154-L174>
safeHistory.added?.forEach((added) => {
prsInHistory.push(added['pr-url'].split('/').at(-1)!);
});

safeHistory.changes?.forEach((change) => {
prsInHistory.push(change['pr-url'].split('/').at(-1)!);
});

safeHistory.deprecated?.forEach((deprecated) => {
prsInHistory.push(deprecated['pr-url'].split('/').at(-1)!);
});

for (const prNumber of prsInHistory) {
if (!allPrReleaseVersions.hasOwnProperty(Number(prNumber))) {
// ? Should this be an error or warning?
console.warn(
`Warning parsing ${filepath}\n
Couldn't find PR number '${prNumber}' in list of all PRs included in releases.\n
Maybe the list is stale? Are you documenting a new change?\n
${JSON.stringify(safeHistory, null, 4)}`,
);
// errorCounter++;
}
}
}

// ? Maybe collect all api history and export it to a single file for future use.
}

Expand Down Expand Up @@ -331,6 +484,7 @@ function parseCommandLine() {
unknown: showUsage,
default: {
'check-placement': true,
// TODO: Change this when GH_TOKEN isn't needed to fetch PR release versions anymore
'check-pull-request-links': false,
'check-strings': true,
},
Expand Down Expand Up @@ -385,6 +539,14 @@ async function init() {
}
}
Comment thread
piotrpdev marked this conversation as resolved.

if (opts['check-pull-request-links'] && process.env.NODE_ENV !== 'test') {
// TODO: Change this when GH_TOKEN isn't needed to fetch PR release versions anymore
if (!process.env.GH_TOKEN) {
console.error('GH_TOKEN environment variable is required for checking pull request links.');
process.exit(1);
}
}

const errors = await main(resolve(process.cwd(), opts.root), opts._, {
checkPlacement: opts['check-placement'],
checkPullRequestLinks: opts['check-pull-request-links'],
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
"homepage": "https://github.com/electron/lint-roller#readme",
"devDependencies": {
"@electron-internal/eslint-config": "^1.0.1",
"@types/adm-zip": "^0.5.5",
"@types/balanced-match": "^1.0.3",
"@types/glob": "^8.1.0",
"@types/markdown-it": "^13.0.6",
Expand All @@ -62,6 +63,7 @@
},
"dependencies": {
"@dsanders11/vscode-markdown-languageservice": "^0.3.0",
"adm-zip": "^0.5.14",
"ajv": "^8.16.0",
"balanced-match": "^2.0.0",
"glob": "^8.1.0",
Expand Down
17 changes: 17 additions & 0 deletions tests/fixtures/api-history-pull-request-invalid.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#### `win.setTrafficLightPosition(position)` _macOS_ _Deprecated_

<!--
```YAML history
added:
- pr-url: https://github.com/electron/electron/pull/225332672
changes:
- pr-url: https://github.com/electron/electron/pull/26789
description: Made `trafficLightPosition` option work for `customButtonOnHover`.
deprecated:
- pr-url: https://github.com/electron/electron/pull/37094
breaking-changes-header: deprecated-browserwindowsettrafficlightpositionposition
```
-->

Set a custom position for the traffic light buttons in frameless window.
Passing `{ x: 0, y: 0 }` will reset the position to default.
Loading