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 script to update VSCode config with proper excludes #110161

Merged
merged 19 commits into from
Aug 30, 2021
Merged

add script to update VSCode config with proper excludes #110161

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
61ade82
add script to update VSCode config with proper excludes
spalger Aug 25, 2021
974b291
resolve paths against repo root
spalger Aug 25, 2021
dd158c1
reduce the scope of kbn/optimizer snapshots to make them less fragile
spalger Aug 26, 2021
ae9fdb7
Merge branch 'master' of github.com:elastic/kibana into implement/set…
spalger Aug 26, 2021
3d07a43
add a few more files to list of watcher excludes
spalger Aug 26, 2021
5a02486
support managing individual settings with `// self managed` comment
spalger Aug 26, 2021
170a30a
Merge branch 'master' of github.com:elastic/kibana into implement/set…
spalger Aug 26, 2021
4330532
Merge branch 'master' of github.com:elastic/kibana into implement/set…
spalger Aug 26, 2021
f4ae606
Merge branch 'master' into implement/setup-vscode-workspace-settings
kibanamachine Aug 30, 2021
f16d1f8
Merge branch 'implement/setup-vscode-workspace-settings' of github.co…
spalger Aug 30, 2021
7c0a63a
expand support for self managing properties
spalger Aug 30, 2021
0685c4b
Merge branch 'master' of github.com:elastic/kibana into implement/set…
spalger Aug 30, 2021
2074e04
update comment of top level function
spalger Aug 30, 2021
83cf3ba
remove existing managed keys which are not objects
spalger Aug 30, 2021
e3a752e
misc cleanup
spalger Aug 30, 2021
b874547
break loop logic up into helper functions for readability
spalger Aug 30, 2021
38d138a
one more rename
spalger Aug 30, 2021
b5b5dd5
handle comment conflicts better and reduce iterations
spalger Aug 30, 2021
acbffcc
I swear, I'm done refactoring
spalger Aug 30, 2021
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
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,7 @@
"devDependencies": {
"@babel/cli": "^7.12.10",
"@babel/core": "^7.12.10",
"@babel/generator": "^7.12.11",
"@babel/parser": "^7.12.11",
"@babel/plugin-proposal-class-properties": "^7.12.1",
"@babel/plugin-proposal-export-namespace-from": "^7.12.1",
Expand Down
2 changes: 2 additions & 0 deletions packages/kbn-dev-utils/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ RUNTIME_DEPS = [
"@npm//load-json-file",
"@npm//markdown-it",
"@npm//normalize-path",
"@npm//prettier",
"@npm//rxjs",
"@npm//tar",
"@npm//tree-kill",
Expand All @@ -81,6 +82,7 @@ TYPES_DEPS = [
"@npm//@types/markdown-it",
"@npm//@types/node",
"@npm//@types/normalize-path",
"@npm//@types/prettier",
"@npm//@types/react",
"@npm//@types/tar",
"@npm//@types/testing-library__jest-dom",
Expand Down
1 change: 1 addition & 0 deletions packages/kbn-dev-utils/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,4 @@ export * from './plugins';
export * from './streams';
export * from './babel';
export * from './extract';
export * from './vscode_config';
9 changes: 9 additions & 0 deletions packages/kbn-dev-utils/src/vscode_config/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

export * from './update_vscode_config_cli';
40 changes: 40 additions & 0 deletions packages/kbn-dev-utils/src/vscode_config/managed_config_keys.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

export interface ManagedConfigKey {
key: string;
value: Record<string, any>;
}

/**
* Defines the keys which we overrite in user's vscode config for the workspace. We currently
* only support object values because that's all we needed to support, but support for non object
* values should be easy to add.
*/
export const MANAGED_CONFIG_KEYS: ManagedConfigKey[] = [
{
key: 'files.watcherExclude',
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if we should list artifacts in search.exclude setting to speed up the default experience for search operations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did you turn off the "Use exclude settings and ignore files" setting? Artifacts should be excluded by default because they're not tracked by git.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I only ask because I've never felt like search was slow and I definitely prefer to rely on the existing ignore files if we can.

Copy link
Member

Choose a reason for hiding this comment

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

One example that may be useful here is the kbn-pm dist. That's the only one I'm running in to on first glance though, doesn't impact me much.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it's checked into the code so it makes sense to me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, in order to support managing the search.exclude property it felt like people needed to be able to add exclusions for themselves, so I've updated the logic to define management comments on each property it writes like so:

/**
 * @managed
 *
 * Some settings in this file are managed by @kbn/dev-utils. When a setting is managed it is preceeded
 * with a comment "// @managed" comment. Replace that with "// self managed" and the scripts will not
 * touch that value. Put a "// self managed" comment at the top of the file, or above a group of settings
 * to disable management of that entire section.
 */
{
  "workbench.colorTheme": "Noctis Obscuro",
  "files.watcherExclude": {
    // self managed
    "**/.eslintcache": false,
    // @managed
    "**/.es": true,
    // @managed
    "**/.yarn-local-mirror": true,
    // @managed
    "**/.chromium": true,
    // @managed
    "**/bazel-*": true,
    // @managed
    "**/node_modules": true,
    // @managed
    "**/target": true,
    // @managed
    "**/*.log": true,
    // @managed
    "**/packages/kbn-pm/dist/index.js": true
  },
  "search.exclude": {
    // @managed
    "**/packages/kbn-pm/dist/index.js": true
  }
}

Now, when the script runs it will ignore any property which doesn't have a @managed comment, and delete @managed properties which are no longer managed. Additionally, users can self manage a property by replacing the @managed comment with // self managed and then customize things further.

value: {
['**/.eslintcache']: true,
['**/.es']: true,
['**/.yarn-local-mirror']: true,
['**/.chromium']: true,
['**/packages/kbn-pm/dist/index.js']: true,
['**/bazel-*']: true,
['**/node_modules']: true,
['**/target']: true,
spalger marked this conversation as resolved.
Show resolved Hide resolved
['**/*.log']: true,
},
},
{
key: 'search.exclude',
value: {
['**/packages/kbn-pm/dist/index.js']: true,
},
},
];
Loading