Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add a pre-commit hook to check whether API docs are updated #18820

Merged
merged 6 commits into from Dec 19, 2019
Merged

Add a pre-commit hook to check whether API docs are updated #18820

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f97e873
Add pre-commit hook for API docs
oandregal Nov 29, 2019
ba13296
Fix typo in message
oandregal Dec 3, 2019
14f3a3b
Add pre-commit hook for handbook data docs
oandregal Dec 9, 2019
31378db
Revert husky change
oandregal Dec 16, 2019
f4aa5ff
Simplify code
oandregal Dec 19, 2019
7790978
Replicate changes from are-readmes-unstaged
oandregal Dec 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/CODEOWNERS
View file
Open in desktop
Validating CODEOWNERS rules …
Expand Up @@ -31,7 +31,7 @@

# Tooling
/bin @ntwb @nerrad @ajitbohra
/bin/update-readmes.js @ntwb @nerrad @ajitbohra @nosolosw
/bin/api-docs @ntwb @nerrad @ajitbohra @nosolosw
/docs/tool @youknowriad @chrisvanpatten @ajitbohra @nosolosw
/packages/babel-plugin-import-jsx-pragma @gziolo @ntwb @nerrad @ajitbohra
/packages/babel-plugin-makepot @ntwb @nerrad @ajitbohra
Expand Down
39 changes: 39 additions & 0 deletions bin/api-docs/are-readmes-unstaged.js
View file
Open in desktop
@@ -0,0 +1,39 @@
#!/usr/bin/env node

/**
* Node dependencies.
*/
const { join } = require( 'path' );
const chalk = require( 'chalk' );
const execSync = require( 'child_process' ).execSync;

/**
* Local dependencies.
*/
const getPackages = require( './packages' );

const getUnstagedFiles = () => execSync( 'git diff --name-only', { encoding: 'utf8' } ).split( '\n' ).filter( ( element ) => '' !== element );

const readmeFiles = getPackages().map( ( [ packageName ] ) => join( 'packages', packageName, 'README.md' ) );
oandregal marked this conversation as resolved.
Show resolved Hide resolved
const unstagedFiles = getUnstagedFiles();

const unstagedReadmes = [];
unstagedFiles.forEach( ( element ) => {
if ( readmeFiles.includes( element ) ) {
unstagedReadmes.push( element );
}
} );
oandregal marked this conversation as resolved.
Show resolved Hide resolved

let exitCode = 0;
if ( unstagedReadmes.length > 0 ) {
exitCode = 1;
oandregal marked this conversation as resolved.
Show resolved Hide resolved
process.stdout.write( chalk.red(
'\n',
'Some API docs may be out of date:',
unstagedReadmes.toString(),
'Either stage them or continue with --no-verify.',
'\n'
) );
}

process.exit( exitCode );
44 changes: 44 additions & 0 deletions bin/api-docs/packages.js
View file
Open in desktop
@@ -0,0 +1,44 @@
const packages = [
'a11y',
'autop',
'blob',
'block-editor',
'block-library',
'block-serialization-default-parser',
'blocks',
'compose',
[ 'core-data', {
'Autogenerated actions': 'src/actions.js',
'Autogenerated selectors': 'src/selectors.js',
} ],
'data',
'data-controls',
'date',
'deprecated',
'dom',
'dom-ready',
'e2e-test-utils',
'edit-post',
'element',
'escape-html',
'html-entities',
'i18n',
'keycodes',
'plugins',
'priority-queue',
'redux-routine',
'rich-text',
'shortcode',
'url',
'viewport',
'wordcount',
];

module.exports = function() {
return packages.map( ( entry ) => {
if ( ! Array.isArray( entry ) ) {
entry = [ entry, { 'Autogenerated API docs': 'src/index.js' } ];
}
return entry;
} );
};
35 changes: 35 additions & 0 deletions bin/api-docs/update-readmes.js
View file
Open in desktop
@@ -0,0 +1,35 @@
/**
* Node dependencies.
*/
const { join } = require( 'path' );
const spawnSync = require( 'child_process' ).spawnSync;

/**
* Local dependencies.
*/
const getPackages = require( './packages' );

getPackages().forEach( ( entry ) => {
const [ packageName, targetFiles ] = entry;

Object.entries( targetFiles ).forEach( ( [ token, path ] ) => {
// Each target operates over the same file, so it needs to be processed synchronously,
// as to make sure the processes don't overwrite each other.
const { status, stderr } = spawnSync(
join( __dirname, '..', '..', 'node_modules', '.bin', 'docgen' ).replace( / /g, '\\ ' ),
[
join( 'packages', packageName, path ),
`--output packages/${ packageName }/README.md`,
'--to-token',
`--use-token "${ token }"`,
'--ignore "/unstable|experimental/i"',
],
{ shell: true },
);

if ( status !== 0 ) {
process.stderr.write( `${ packageName } ${ stderr.toString() }\n` );
process.exit( 1 );
}
} );
} );
70 changes: 0 additions & 70 deletions bin/update-readmes.js
View file
Open in desktop

This file was deleted.

38 changes: 38 additions & 0 deletions docs/tool/are-data-files-unstaged.js
View file
Open in desktop
@@ -0,0 +1,38 @@
#!/usr/bin/env node

/**
* Node dependencies.
*/
const chalk = require( 'chalk' );
const execSync = require( 'child_process' ).execSync;

/**
* Local dependencies.
*/
const getPackages = require( './packages' );

const getUnstagedFiles = () => execSync( 'git diff --name-only', { encoding: 'utf8' } ).split( '\n' ).filter( ( element ) => '' !== element );

const readmeFiles = getPackages().map( ( [ packageName ] ) => `docs/designers-developers/developers/data/data-${ packageName.replace( '/', '-' ) }.md` );
const unstagedFiles = getUnstagedFiles();

const unstagedReadmes = [];
unstagedFiles.forEach( ( element ) => {
if ( readmeFiles.includes( element ) ) {
unstagedReadmes.push( element );
}
} );

let exitCode = 0;
if ( unstagedReadmes.length > 0 ) {
exitCode = 1;
process.stdout.write( chalk.red(
'\n',
'Some API docs may be out of date:',
unstagedReadmes.toString(),
'Either stage them or continue with --no-verify.',
'\n'
) );
}

process.exit( exitCode );
26 changes: 26 additions & 0 deletions docs/tool/packages.js
View file
Open in desktop
@@ -0,0 +1,26 @@
const packages = [
[ 'core', {
'Autogenerated actions': 'packages/core-data/src/actions.js',
'Autogenerated selectors': 'packages/core-data/src/selectors.js',
} ],
'core/annotations',
'core/blocks',
'core/block-editor',
'core/editor',
'core/edit-post',
'core/notices',
'core/nux',
'core/viewport',
];

module.exports = function() {
return packages.map( ( entry ) => {
if ( ! Array.isArray( entry ) ) {
entry = [ entry, {
'Autogenerated actions': `packages/${ entry.replace( 'core/', '' ) }/src/store/actions.js`,
'Autogenerated selectors': `packages/${ entry.replace( 'core/', '' ) }/src/store/selectors.js`,
} ];
}
return entry;
} );
};
35 changes: 10 additions & 25 deletions docs/tool/update-data.js
View file
Open in desktop
Expand Up @@ -4,39 +4,23 @@
const { join } = require( 'path' );
const spawnSync = require( 'child_process' ).spawnSync;

const modules = [
[ 'core', {
'Autogenerated actions': 'packages/core-data/src/actions.js',
'Autogenerated selectors': 'packages/core-data/src/selectors.js',
} ],
'core/annotations',
'core/blocks',
'core/block-editor',
'core/editor',
'core/edit-post',
'core/notices',
'core/nux',
'core/viewport',
];
/**
* Local dependencies.
*/
const getPackages = require( './packages' );

modules.forEach( ( entry ) => {
if ( ! Array.isArray( entry ) ) {
entry = [ entry, {
'Autogenerated actions': `packages/${ entry.replace( 'core/', '' ) }/src/store/actions.js`,
'Autogenerated selectors': `packages/${ entry.replace( 'core/', '' ) }/src/store/selectors.js`,
} ];
}
const [ namespace, targets ] = entry;
getPackages().forEach( ( entry ) => {
const [ packageName, targetFiles ] = entry;

Object.entries( targets ).forEach( ( [ token, target ] ) => {
Object.entries( targetFiles ).forEach( ( [ token, target ] ) => {
// Note that this needs to be a sync process for each output file that is updated:
// until docgen provides a way to update many tokens at once, we need to make sure
// the output file is updated before starting the second pass for the next token.
const { status, stderr } = spawnSync(
join( __dirname, '..', '..', 'node_modules', '.bin', 'docgen' ).replace( / /g, '\\ ' ),
[
target,
`--output docs/designers-developers/developers/data/data-${ namespace.replace( '/', '-' ) }.md`,
`--output docs/designers-developers/developers/data/data-${ packageName.replace( '/', '-' ) }.md`,
'--to-token',
`--use-token "${ token }"`,
'--ignore "/unstable|experimental/i"',
Expand All @@ -45,7 +29,8 @@ modules.forEach( ( entry ) => {
);

if ( status !== 0 ) {
throw stderr.toString();
process.stderr.write( `${ packageName } ${ stderr.toString() }\n` );
process.exit( 1 );
}
} );
} );
10 changes: 6 additions & 4 deletions package.json
View file
Open in desktop
Expand Up @@ -120,7 +120,7 @@
"fast-glob": "2.2.7",
"fbjs": "0.8.17",
"glob": "7.1.2",
"husky": "3.0.5",
"husky": "2.7.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was it intentional to downgrade? I would expect it would need corresponding changes to package-lock.json if intended.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to revert this.

For context: when we updated husky from 0 to 3 we introduced a breaking change: the git version required to work with husky is >= 2.13.2. Git hooks won't be executed in versions below. The git version bundled with my OS is lower than that, so I can't run the hooks (and test this change), so I had to downgrade it for testing.

I asked in core-editor about this change and it doesn't seem a widespread issue. Wasn't able to pin down easily the git versions that come with the supported OS for Windows and Mac to gauge how many could be affected. I guessed another way to look at it was that if people don't complain about hooks not executing for them, it's not an issue.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

31378db

"inquirer": "6.3.1",
"is-equal-shallow": "0.1.3",
"jest-junit": "6.4.0",
Expand Down Expand Up @@ -171,7 +171,7 @@
"predev": "npm run check-engines",
"dev": "npm run build:packages && concurrently \"wp-scripts start\" \"npm run dev:packages\"",
"dev:packages": "node ./bin/packages/watch.js",
"docs:build": "node ./docs/tool/index.js && node ./bin/update-readmes.js",
"docs:build": "node ./docs/tool/index.js && node ./bin/api-docs/update-readmes.js",
"fixtures:clean": "rimraf \"packages/e2e-tests/fixtures/blocks/*.+(json|serialized.html)\"",
"fixtures:server-registered": "wp-scripts env docker-run php ./bin/get-server-blocks.php > test/integration/full-content/server-registered.json",
"fixtures:generate": "npm run fixtures:server-registered && cross-env GENERATE_MISSING_FIXTURES=y npm run test-unit",
Expand Down Expand Up @@ -228,10 +228,12 @@
"wp-scripts lint-js"
],
"{docs/{toc.json,tool/*.js},packages/{*/README.md,*/src/{actions,selectors}.js,components/src/*/**/README.md}}": [
"node ./docs/tool/index.js"
"node ./docs/tool/index.js",
"node ./docs/tool/are-data-files-unstaged.js"
],
"packages/**/*.js": [
"node ./bin/update-readmes.js"
"node ./bin/api-docs/update-readmes.js",
"node ./bin/api-docs/are-readmes-unstaged.js"
]
},
"wp-env": {
Expand Down