Skip to content

Commit

Permalink
🏗 Shorter, uniform ignore lists (ampproject#36357)
Browse files Browse the repository at this point in the history
Fixes ampproject#36354

### `amp clean`

- Uses `.gitignore` as source of truth

- Finds paths recursively like `.gitignore` does:

    > #### [Pattern format][1]
    > [1]: https://git-scm.com/docs/gitignore#_pattern_format
    >
    > If there is a separator at the beginning or middle (or both) of the pattern, then the pattern is relative to the directory level of the particular `.gitignore` file itself. Otherwise the pattern may also match at any level below the `.gitignore` level.

### `amp check-ignore-list`

- Ensures that `.gitignore`, `.eslintignore` and `.prettierignore` stay in sync
- Runs during CI checks.

### `.*ignore` files

- Use a shorter list to take advantage of recursive behavior
- Added header to determine uniform section
  • Loading branch information
alanorozco authored Oct 18, 2021
1 parent 6feea0e commit 530a457
Show file tree
Hide file tree
Showing 8 changed files with 168 additions and 130 deletions.
50 changes: 18 additions & 32 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -1,46 +1,32 @@
# Local cache directories
# Keep this list in sync with .gitignore, .prettierignore, and build-system/tasks/clean.js
.babel-cache
.css-cache
.jss-cache
.pre-closure-cache
# Files and directories explicitly ignored by eslint
**/node_modules/**
build-system/babel-plugins/**/fixtures/**/*.js
build-system/babel-plugins/**/fixtures/**/*.mjs
build-system/tasks/make-extension/template/**/*
examples/amp-script/todomvc.ssr.js
examples/amp-script/vue-todomvc.js
extensions/amp-a4a/0.1/test/testdata/**
src/purifier/noop.js
testing/local-amp-chrome-extension/**
third_party/**
validator/**

# [GLOBALLY IGNORED]
# (Don't edit or remove the above comment!)
# The rules below are synced between .gitignore and .prettierignore, and are
# removed by `amp clean`.

# Output directories
# Keep this list in sync with .gitignore, .prettierignore, and build-system/tasks/clean.js
.*cache
.amp-dep-check
build
build-system/dist
build-system/server/new-server/transforms/dist
build-system/tasks/performance/cache
build-system/tasks/performance/results.json
dist
dist.3p
dist.tools
export
examples/storybook
extensions/**/dist
/release
result-reports
src/purifier/dist
test/coverage
test/coverage-e2e
validator/**/dist
validator/export

# User configuration files
# Keep this list in sync with .gitignore, .prettierignore, and build-system/tasks/clean.js
build-system/global-configs/custom-config.json
build-system/global-configs/custom-flavors-config.json

# Files and directories explicitly ignored by eslint
**/node_modules/**
build-system/babel-plugins/**/fixtures/**/*.js
build-system/babel-plugins/**/fixtures/**/*.mjs
build-system/tasks/make-extension/template/**/*
examples/amp-script/todomvc.ssr.js
examples/amp-script/vue-todomvc.js
extensions/amp-a4a/0.1/test/testdata/**
src/purifier/noop.js
testing/local-amp-chrome-extension/**
third_party/**
validator/**
59 changes: 25 additions & 34 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,37 +1,3 @@
# Local cache directories
# Keep this list in sync with .eslintignore, .prettierignore, and build-system/tasks/clean.js
.babel-cache
.css-cache
.jss-cache
.pre-closure-cache

# Output directories
# Keep this list in sync with .eslintignore, .prettierignore, and build-system/tasks/clean.js
.amp-dep-check
build
build-system/dist
build-system/server/new-server/transforms/dist
build-system/tasks/performance/cache
build-system/tasks/performance/results.json
dist
dist.3p
dist.tools
export
examples/storybook
extensions/**/dist
/release
result-reports
src/purifier/dist
test/coverage
test/coverage-e2e
validator/**/dist
validator/export

# User configuration files
# Keep this list in sync with .eslintignore, .prettierignore, and build-system/tasks/clean.js
build-system/global-configs/custom-config.json
build-system/global-configs/custom-flavors-config.json

# OS level files
.DS_Store
.g4ignore
Expand All @@ -47,3 +13,28 @@ npm-debug.log
.firebase
firebase
firebase.json

# User configuration files
# Keep this list in sync with clean.js
/build-system/global-configs/custom-config.json
/build-system/global-configs/custom-flavors-config.json

# [GLOBALLY IGNORED]
# (Don't edit or remove the above comment!)
# The rules below are synced between .gitignore and .prettierignore, and are
# removed by `amp clean`.

.*cache
.amp-dep-check
build
build-system/tasks/performance/cache
build-system/tasks/performance/results.json
dist
dist.3p
dist.tools
export
examples/storybook
/release
result-reports
test/coverage
test/coverage-e2e
32 changes: 9 additions & 23 deletions .prettierignore
Original file line number Diff line number Diff line change
@@ -1,37 +1,23 @@
# Local cache directories
# Keep this list in sync with .gitignore, .eslintignore, and build-system/tasks/clean.js
.babel-cache
.css-cache
.jss-cache
.pre-closure-cache
# Files and directories explicitly ignored
**/package*.json
**/node_modules/**

# Output directories
# Keep this list in sync with .gitignore, .eslintignore, and build-system/tasks/clean.js
# [GLOBALLY IGNORED]
# (Don't edit or remove the above comment!)
# The rules below are synced between .gitignore and .prettierignore, and are
# removed by `amp clean`.

.*cache
.amp-dep-check
build
build-system/dist
build-system/server/new-server/transforms/dist
build-system/tasks/performance/cache
build-system/tasks/performance/results.json
dist
dist.3p
dist.tools
export
examples/storybook
extensions/**/dist
/release
result-reports
src/purifier/dist
test/coverage
test/coverage-e2e
validator/**/dist
validator/export

# User configuration files
# Keep this list in sync with .gitignore, .eslintignore, and build-system/tasks/clean.js
build-system/global-configs/custom-config.json
build-system/global-configs/custom-flavors-config.json

# Files and directories explicitly ignored by prettier
**/package*.json
**/node_modules/**
1 change: 1 addition & 0 deletions amp.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ createTask('check-analytics-vendors-list', 'checkAnalyticsVendorsList');
createTask('check-asserts', 'checkAsserts');
createTask('check-build-system', 'checkBuildSystem');
createTask('check-exact-versions', 'checkExactVersions');
createTask('check-ignore-lists', 'checkIgnoreLists');
createTask('check-invalid-whitespaces', 'checkInvalidWhitespaces');
createTask('check-links', 'checkLinks');
createTask('check-owners', 'checkOwners');
Expand Down
10 changes: 10 additions & 0 deletions build-system/pr-check/build-targets.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const path = require('path');
const {cyan} = require('kleur/colors');
const {getLoggingPrefix, logWithoutTimestamp} = require('../common/logging');
const {gitDiffNameOnlyMain} = require('../common/git');
const {ignoreListFiles} = require('../tasks/check-ignore-lists');
const {isCiBuild} = require('../common/ci');

/**
Expand All @@ -38,6 +39,7 @@ const Targets = {
DOCS: 'DOCS',
E2E_TEST: 'E2E_TEST',
HTML_FIXTURES: 'HTML_FIXTURES',
IGNORE_LIST: 'IGNORE_LIST',
INTEGRATION_TEST: 'INTEGRATION_TEST',
INVALID_WHITESPACES: 'INVALID_WHITESPACES',
LINT: 'LINT',
Expand Down Expand Up @@ -66,6 +68,7 @@ const nonRuntimeTargets = [
Targets.DEV_DASHBOARD,
Targets.DOCS,
Targets.E2E_TEST,
Targets.IGNORE_LIST,
Targets.INTEGRATION_TEST,
Targets.OWNERS,
Targets.RENOVATE_CONFIG,
Expand Down Expand Up @@ -188,6 +191,13 @@ const targetMatchers = {
file.startsWith('build-system/test-configs')
);
},
[Targets.IGNORE_LIST]: (file) => {
return (
ignoreListFiles.includes(file) ||
file === 'build-system/tasks/check-ignore-list.js' ||
file === 'build-system/tasks/clean.js'
);
},
[Targets.INTEGRATION_TEST]: (file) => {
if (isOwnersFile(file)) {
return false;
Expand Down
5 changes: 5 additions & 0 deletions build-system/pr-check/checks.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ function pushBuildWorkflow() {
timedExecOrDie('amp prettify');
timedExecOrDie('amp ava');
timedExecOrDie('amp check-build-system');
timedExecOrDie('amp check-ignore-list');
timedExecOrDie('amp babel-plugin-tests');
timedExecOrDie('amp caches-json');
timedExecOrDie('amp check-exact-versions');
Expand Down Expand Up @@ -50,6 +51,10 @@ function prBuildWorkflow() {
timedExecOrDie('amp check-invalid-whitespaces');
}

if (buildTargetsInclude(Targets.IGNORE_LIST)) {
timedExecOrDie(`amp check-ignore-lists`);
}

if (buildTargetsInclude(Targets.HTML_FIXTURES)) {
timedExecOrDie('amp validate-html-fixtures');
}
Expand Down
44 changes: 44 additions & 0 deletions build-system/tasks/check-ignore-lists.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
const {readFile} = require('fs-extra');
const {writeDiffOrFail} = require('../common/diff');

const ignoreFile = '.gitignore';
const syncIgnoreFiles = ['.prettierignore', '.eslintignore'];

const ignoreListFiles = [ignoreFile, ...syncIgnoreFiles];

const header = '# [GLOBALLY IGNORED]';

/**
* @param {string} source
* @return {string[]}
*/
function splitIgnoreListByHeader(source) {
return source.split(header, 2);
}

/**
* @return {!Promise}
*/
async function checkIgnoreLists() {
const [, below] = splitIgnoreListByHeader(await readFile(ignoreFile, 'utf8'));

for (const file of syncIgnoreFiles) {
const [above] = splitIgnoreListByHeader(await readFile(file, 'utf8'));
const tentative = [above, below].join(header);
await writeDiffOrFail('check-ignore-list', file, tentative);
}
}

checkIgnoreLists.description =
'Check ignore lists to make sure they are in sync';

checkIgnoreLists.flags = {
'fix': `Fix files out of sync`,
};

module.exports = {
checkIgnoreLists,
ignoreListFiles,
ignoreFile,
splitIgnoreListByHeader,
};
Loading

0 comments on commit 530a457

Please sign in to comment.