ci: check code-ownership on CI (#32577)

This commit expands the `lint` CircleCI job to also run the
`tools/verify-codeownership.js` script. This script verifies that some
important files/directories in the codebase have code-owners assigned in
`.github/CODEOWNERS`.

The main purpose of this change is to prevent adding new directories
(e.g. packages or docs guides/examples) without assigning appropriate
code-owners. When no code-owner is explicitly assigned, corresponding
PRs will automatically request reviews from @igorminar, who is the
"fall-back" code-owner.

PR Close #32577
This commit is contained in:
George Kalpakas 2019-09-10 13:56:38 +03:00 committed by Matias Niemelä
parent d0dd69f177
commit 65f5c0476f
2 changed files with 54 additions and 36 deletions

View File

@ -234,6 +234,7 @@ jobs:
(echo -e "\n.bzl files have lint errors. Please run ''yarn bazel:lint-fix''"; exit 1)' (echo -e "\n.bzl files have lint errors. Please run ''yarn bazel:lint-fix''"; exit 1)'
- run: yarn gulp lint - run: yarn gulp lint
- run: node tools/verify-codeownership
test: test:
<<: *job_defaults <<: *job_defaults

View File

@ -1,17 +1,24 @@
#!/usr/bin/env node /**
'use strict'; * @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
/** /**
* **Usage:** * **Usage:**
* ``` * ```
* node aio/scripts/verify-codeownership * node tools/verify-codeownership
* ``` * ```
* *
* Verify whether there are directories in the codebase that don't have a codeowner (in `.github/CODEOWNERS`) and vice * Verify whether there are directories in the codebase that don't have a codeowner (in
* versa (that there are no patterns in `CODEOWNERS` that do not correspond to actual directories). * `.github/CODEOWNERS`) and vice versa (that there are no patterns in `CODEOWNERS` that do not
* correspond to actual directories).
* *
* The script does not aim to be exhaustive and highly accurate, checking all files and directories (since that would be * The script does not aim to be exhaustive and highly accurate, checking all files and directories
* too complicated). Instead, it does a coarse check on some important (or frequently changing) directories. * (since that would be too complicated). Instead, it does a coarse check on some important (or
* frequently changing) directories.
* *
* Currently, it checks the following: * Currently, it checks the following:
* - **Packages**: Top-level directories in `packages/`. * - **Packages**: Top-level directories in `packages/`.
@ -20,13 +27,15 @@
* - **Guide images**: Top-level directories in `aio/content/images/guide/`. * - **Guide images**: Top-level directories in `aio/content/images/guide/`.
* - **Guide examples**: Top-level directories in `aio/content/examples/`. * - **Guide examples**: Top-level directories in `aio/content/examples/`.
*/ */
'use strict';
// Imports // Imports
const chalk = require('chalk');
const fs = require('fs'); const fs = require('fs');
const path = require('path'); const path = require('path');
// Constants // Constants
const PROJECT_ROOT_DIR = path.resolve(__dirname, '../..'); const PROJECT_ROOT_DIR = path.resolve(__dirname, '..');
const CODEOWNERS_PATH = path.resolve(PROJECT_ROOT_DIR, '.github/CODEOWNERS'); const CODEOWNERS_PATH = path.resolve(PROJECT_ROOT_DIR, '.github/CODEOWNERS');
const PKG_DIR = path.resolve(PROJECT_ROOT_DIR, 'packages'); const PKG_DIR = path.resolve(PROJECT_ROOT_DIR, 'packages');
const PKG_EXAMPLES_DIR = path.resolve(PKG_DIR, 'examples'); const PKG_EXAMPLES_DIR = path.resolve(PKG_DIR, 'examples');
@ -45,7 +54,11 @@ _main();
// Functions - Definitions // Functions - Definitions
function _main() { function _main() {
const {packages: pkgPackagePaths, examples: pkgExamplePaths} = getPathsFromPkg(); const {packages: pkgPackagePaths, examples: pkgExamplePaths} = getPathsFromPkg();
const {guides: aioGuidePaths, images: aioGuideImagesPaths, examples: aioExamplePaths} = getPathsFromAioContent(); const {
guides: aioGuidePaths,
images: aioGuideImagesPaths,
examples: aioExamplePaths,
} = getPathsFromAioContent();
const { const {
pkgPackages: coPkgPackagePaths, pkgPackages: coPkgPackagePaths,
pkgExamples: coPkgExamplePaths, pkgExamples: coPkgExamplePaths,
@ -60,7 +73,8 @@ function _main() {
const aioImagesDiff = arrayDiff(aioGuideImagesPaths, coAioGuideImagesPaths); const aioImagesDiff = arrayDiff(aioGuideImagesPaths, coAioGuideImagesPaths);
const aioExamplesDiff = arrayDiff(aioExamplePaths, coAioExamplePaths); const aioExamplesDiff = arrayDiff(aioExamplePaths, coAioExamplePaths);
const hasDiff = (pkgPackagesDiff.diffCount > 0) || (pkgExamplesDiff.diffCount > 0) || const hasDiff = (pkgPackagesDiff.diffCount > 0) || (pkgExamplesDiff.diffCount > 0) ||
(aioGuidesDiff.diffCount > 0) || (aioImagesDiff.diffCount> 0) || (aioExamplesDiff.diffCount > 0); (aioGuidesDiff.diffCount > 0) || (aioImagesDiff.diffCount > 0) ||
(aioExamplesDiff.diffCount > 0);
if (hasDiff) { if (hasDiff) {
const expectedPkgPackagesSrc = path.relative(PROJECT_ROOT_DIR, PKG_DIR); const expectedPkgPackagesSrc = path.relative(PROJECT_ROOT_DIR, PKG_DIR);
@ -75,6 +89,16 @@ function _main() {
reportDiff(aioGuidesDiff, expectedAioGuidesSrc, actualSrc); reportDiff(aioGuidesDiff, expectedAioGuidesSrc, actualSrc);
reportDiff(aioImagesDiff, expectedAioImagesSrc, actualSrc); reportDiff(aioImagesDiff, expectedAioImagesSrc, actualSrc);
reportDiff(aioExamplesDiff, expectedAioExamplesSrc, actualSrc); reportDiff(aioExamplesDiff, expectedAioExamplesSrc, actualSrc);
// tslint:disable-next-line: no-console
console.log(chalk.red(
'\nCode-ownership verification failed.\n' +
'Please update \'.github/CODEOWNERS\' to ensure that all necessary files/directories ' +
'have code-owners and all patterns that appear in the file correspond to actual ' +
'files/directories in the repo.'));
} else {
// tslint:disable-next-line: no-console
console.log(chalk.green('\nCode-ownership verification succeeded!'));
} }
process.exit(hasDiff ? 1 : 0); process.exit(hasDiff ? 1 : 0);
@ -88,16 +112,16 @@ function arrayDiff(expected, actual) {
} }
function findDirectories(parentDir) { function findDirectories(parentDir) {
return fs.readdirSync(parentDir). return fs.readdirSync(parentDir).filter(
filter(name => fs.statSync(`${parentDir}/${name}`).isDirectory()); name => fs.statSync(`${parentDir}/${name}`).isDirectory());
} }
function getPathsFromAioContent() { function getPathsFromAioContent() {
return { return {
guides: fs.readdirSync(AIO_GUIDES_DIR), guides: fs.readdirSync(AIO_GUIDES_DIR),
images: fs.readdirSync(AIO_GUIDE_IMAGES_DIR), images: fs.readdirSync(AIO_GUIDE_IMAGES_DIR),
examples: fs.readdirSync(AIO_GUIDE_EXAMPLES_DIR). examples: fs.readdirSync(AIO_GUIDE_EXAMPLES_DIR)
filter(name => fs.statSync(`${AIO_GUIDE_EXAMPLES_DIR}/${name}`).isDirectory()), .filter(name => fs.statSync(`${AIO_GUIDE_EXAMPLES_DIR}/${name}`).isDirectory()),
}; };
} }
@ -106,7 +130,8 @@ function getPathsFromCodeowners() {
const pkgExamplesPathRe = /^\/packages\/examples\/([^\s\*/]+)/; const pkgExamplesPathRe = /^\/packages\/examples\/([^\s\*/]+)/;
// Use capturing groups for `images/` and `examples` to be able to differentiate between the // Use capturing groups for `images/` and `examples` to be able to differentiate between the
// different kinds of matches (guide, image, example) later (see `isImage`/`isExample` below). // different kinds of matches (guide, image, example) later (see `isImage`/`isExample` below).
const aioGuidesImagesExamplesPathRe = /^\/aio\/content\/(?:(images\/)?guide|(examples))\/([^\s\*/]+)/; const aioGuidesImagesExamplesPathRe =
/^\/aio\/content\/(?:(images\/)?guide|(examples))\/([^\s\*/]+)/;
const manualGlobExpansions = { const manualGlobExpansions = {
// `CODEOWNERS` has a glob to match all `testing/` directories, so no specific glob for // `CODEOWNERS` has a glob to match all `testing/` directories, so no specific glob for
// `packages/examples/testing/` is necessary. // `packages/examples/testing/` is necessary.
@ -120,10 +145,7 @@ function getPathsFromCodeowners() {
const aioExamples = []; const aioExamples = [];
// Read `CODEOWNERS` and split into lines. // Read `CODEOWNERS` and split into lines.
const lines = fs. const lines = fs.readFileSync(CODEOWNERS_PATH, 'utf8').split('\n').map(l => l.trim());
readFileSync(CODEOWNERS_PATH, 'utf8').
split('\n').
map(l => l.trim());
// Manually expand globs to known matching patterns. // Manually expand globs to known matching patterns.
for (const [glob, expansions] of Object.entries(manualGlobExpansions)) { for (const [glob, expansions] of Object.entries(manualGlobExpansions)) {
@ -134,27 +156,22 @@ function getPathsFromCodeowners() {
} }
// Collect packages (`packages/`). // Collect packages (`packages/`).
lines. lines.map(l => l.match(pkgPackagesPathRe)).filter(m => m).forEach(([
map(l => l.match(pkgPackagesPathRe)). , path
filter(m => m). ]) => pkgPackages.push(path));
forEach(([, path]) => pkgPackages.push(path));
// Collect API docs examples (`packages/examples/`). // Collect API docs examples (`packages/examples/`).
lines. lines.map(l => l.match(pkgExamplesPathRe)).filter(m => m).forEach(([
map(l => l.match(pkgExamplesPathRe)). , path
filter(m => m). ]) => pkgExamples.push(path));
forEach(([, path]) => pkgExamples.push(path));
// Collect `aio/` guides/images/examples. // Collect `aio/` guides/images/examples.
lines. lines.map(l => l.match(aioGuidesImagesExamplesPathRe))
map(l => l.match(aioGuidesImagesExamplesPathRe)). .filter(m => m)
filter(m => m). .forEach(([, isImage, isExample, path]) => {
forEach(([, isImage, isExample, path]) => { const list = isExample ? aioExamples : isImage ? aioImages : aioGuides;
const list = isExample ? aioExamples : list.push(path);
isImage ? aioImages : });
aioGuides;
list.push(path);
});
return {pkgPackages, pkgExamples, aioGuides, aioImages, aioExamples}; return {pkgPackages, pkgExamples, aioGuides, aioImages, aioExamples};
} }