From ff15cdf9bfd642207c24229622c230baecea3f3b Mon Sep 17 00:00:00 2001 From: Raghu Simha Date: Fri, 18 Oct 2019 13:06:33 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=8F=97=20Add=20VS=20Code=20auto-formattin?= =?UTF-8?q?g=20support=20for=20JS=20and=20non-JS=20files=20(#25117)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/OWNERS | 9 +++---- .gitignore | 1 - .vscode/OWNERS | 10 +++++++ .vscode/settings.json | 28 ++++++++++++++++++++ build-system/common/utils.js | 2 +- build-system/tasks/prettify.js | 9 ++++++- build-system/test-configs/config.js | 7 +++++ contributing/getting-started-e2e.md | 41 +++++++++++++++++++++++++---- 8 files changed, 93 insertions(+), 14 deletions(-) create mode 100644 .vscode/OWNERS create mode 100644 .vscode/settings.json diff --git a/.github/OWNERS b/.github/OWNERS index 68c6d6f592288..643f230b57d4e 100644 --- a/.github/OWNERS +++ b/.github/OWNERS @@ -4,10 +4,7 @@ { rules: [ { - owners: [ - { name: 'ampproject/wg-outreach' }, - { name: 'ampproject/wg-infra' } - ] - } - ] + owners: [{name: 'ampproject/wg-outreach'}, {name: 'ampproject/wg-infra'}], + }, + ], } diff --git a/.gitignore b/.gitignore index cb90484df1604..b667d93a3274d 100644 --- a/.gitignore +++ b/.gitignore @@ -12,7 +12,6 @@ npm-debug.log .idea .tm_properties .settings -.vscode typings typings.json build-system/runner/dist diff --git a/.vscode/OWNERS b/.vscode/OWNERS new file mode 100644 index 0000000000000..8425845cae3ee --- /dev/null +++ b/.vscode/OWNERS @@ -0,0 +1,10 @@ +// For an explanation of the OWNERS rules and syntax, see: +// https://github.com/ampproject/amp-github-apps/blob/master/owners/OWNERS.example + +{ + rules: [ + { + owners: [{name: 'ampproject/wg-infra'}, {name: 'rsimha', notify: true}], + }, + ], +} diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 0000000000000..a6e8184eb1fef --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,28 @@ +{ + "files.associations": { + // Enable JSON5 syntax highlighting and auto-formatting for OWNERS files. + // Until VS Code adds native JSON5 support, this needs + // https://marketplace.visualstudio.com/items?itemName=mrmlnc.vscode-json5 + "OWNERS": "json5", + + // Enable JSON auto-formatting for these files. + ".eslintrc": "json", + ".prettierrc": "json", + ".renovaterc.json": "json", + ".codecov.yml": "yaml", + ".lando.yml": "yaml", + ".lgtm.yml": "yaml", + ".travis.yml": "yaml" + }, + + // Auto-fix JS files with ESLint using amphtml's custom settings. Needs + // https://marketplace.visualstudio.com/items?itemName=dbaeumer.vscode-eslint + "eslint.autoFixOnSave": true, + "[js]": {"editor.formatOnSave": false}, + + // Auto-fix non-JS files with Prettier using amphtml's custom settings. Needs + // https://marketplace.visualstudio.com/items?itemName=esbenp.prettier-vscode + "[yaml]": {"editor.formatOnSave": true}, + "[json]": {"editor.formatOnSave": true}, + "[json5]": {"editor.formatOnSave": true} +} diff --git a/build-system/common/utils.js b/build-system/common/utils.js index f3b0b4b0b2b67..083f0142a5f0f 100644 --- a/build-system/common/utils.js +++ b/build-system/common/utils.js @@ -42,7 +42,7 @@ function logOnSameLine(message) { * @return {!Array} */ function getFilesChanged(globs) { - const allFiles = globby.sync(globs); + const allFiles = globby.sync(globs, {dot: true}); return gitDiffNameOnlyMaster().filter(changedFile => { return fs.existsSync(changedFile) && allFiles.includes(changedFile); }); diff --git a/build-system/tasks/prettify.js b/build-system/tasks/prettify.js index 4da50a320d728..3fb1a0fcbd297 100644 --- a/build-system/tasks/prettify.js +++ b/build-system/tasks/prettify.js @@ -77,7 +77,7 @@ async function prettify() { logFiles(filesToCheck); } } else { - filesToCheck = globby.sync(prettifyGlobs); + filesToCheck = globby.sync(prettifyGlobs, {dot: true}); } runPrettify(filesToCheck); } @@ -107,6 +107,13 @@ function runPrettify(filesToCheck) { 'Since this is a destructive operation (that edits your files', 'in-place), make sure you commit before running the command.' ); + log( + yellow('NOTE 3:'), + 'For more information, read', + cyan( + 'https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#code-quality-and-style' + ) + ); } } else { if (!isTravisBuild()) { diff --git a/build-system/test-configs/config.js b/build-system/test-configs/config.js index 5802b4bf85b8d..60dcc29f7e498 100644 --- a/build-system/test-configs/config.js +++ b/build-system/test-configs/config.js @@ -139,6 +139,12 @@ const presubmitGlobs = [ '!firebase/**/*.*', ]; +/** + * List of non-JS files to be checked by `gulp prettify` (using prettier). + * NOTE: When you add a new filename / glob to this list: + * 1. Make sure its formatting options are specified in .prettierrc + * 2. Make sure it is listed in .vscode/settings.json (for auto-fix-on-save) + */ const prettifyGlobs = [ '.codecov.yml', '.lando.yml', @@ -147,6 +153,7 @@ const prettifyGlobs = [ '**/.eslintrc', '.prettierrc', '.renovaterc.json', + '.vscode/settings.json', '**/*.json', '**/OWNERS', '!**/{node_modules,build,dist,dist.3p,dist.tools}/**', diff --git a/contributing/getting-started-e2e.md b/contributing/getting-started-e2e.md index 612ee112aa9d0..c14390898e994 100644 --- a/contributing/getting-started-e2e.md +++ b/contributing/getting-started-e2e.md @@ -352,17 +352,48 @@ Note that you *can* add changes into an existing commit but that opens up some a ## Code quality and style -AMP uses [Eslint](https://eslint.org/) to ensure code quality and [Prettier](https://prettier.io/) to standardize code style. For easy development, here are two recommendations: -- Use a code editor with Eslint support to make sure that your code satisfies all of AMP's quality and style rules. [Here](https://eslint.org/docs/user-guide/integrations#editors) is a list of editors with Eslint extension support. -- Set your editor to automatically fix Eslint errors in your code on save. +AMP uses the following tools for code quality and style: +- [Eslint](https://eslint.org/) is used to ensure the code quality of JS files. + - Default rules can be found in [.eslintrc](../.eslintrc) files across the repo. + - Custom rules can be found in [build-system/eslint-rules/](../build-system/eslint-rules/index.js). +- [Prettier](https://prettier.io/) is used to standardize the code style and formatting of JS files and several non-JS files. + - Default and file-specific rules can be found in [.prettierrc](../.prettierrc). -For example, if you use [Visual Studio Code](https://code.visualstudio.com/), you can install its [Eslint plugin](https://marketplace.visualstudio.com/items?itemName=dbaeumer.vscode-eslint), and enable the `eslint.autoFixOnSave` setting. +To easily ensure code quality and style during development, here are some recommendations: +- Use a code editor with Eslint and Prettier support. +- [Here](https://eslint.org/docs/user-guide/integrations#editors) is a list of editors with Eslint extension support. +- [Here](https://prettier.io/docs/en/editors.html) is a list of editors with Prettier extension support. +- Set your editor to automatically format your code on save. -Alternatively, you can manually fix lint errors in your code by running: +### Workflow for Visual Studio Code + +Several AMP developers use [Visual Studio Code](https://code.visualstudio.com/). Here is the recommended workflow to automatically fix your code as you edit it: + +#### To automatically fix JS files on save + +- Install the [Eslint plugin](https://marketplace.visualstudio.com/items?itemName=dbaeumer.vscode-eslint). +- The `eslint.autoFixOnSave` setting is already enabled for the project in [.vscode/settings.json](../.vscode/settings.json), and will cause all JS files to automatically get formatted on save. + +#### To automatically fix non-JS files on save + +- Install the [Prettier plugin](https://marketplace.visualstudio.com/items?itemName=esbenp.prettier-vscode). +- Install the [JSON5 syntax plugin](https://marketplace.visualstudio.com/items?itemName=mrmlnc.vscode-json5). (Used by `OWNERS` files. VS Code does not natively support it.) +- Language and file level settings are already enabled for the project in [.vscode/settings.json](../.vscode/settings.json), and will cause several non-JS files to automatically get formatted on save. + +### Manually fixing code + +Alternatively, you can manually fix most quality and style errors in your code by running these two commands before pushing commits to your GitHub branch. (These commands edit your files in-place, so make sure you commit before running them.) + +For JS files: ``` gulp lint --local_changes --fix ``` +For non-JS files: +``` +gulp prettify --local_changes --fix +``` + # Testing your changes Before sending your code changes for review, please make sure that all of the affected tests are passing. You may be expected to add/modify some tests as well.