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.

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

[APM] Script optimization of APM-specific tsconfig #49868

Merged
merged 3 commits into from
Nov 11, 2019

Conversation

dgieselaar
Copy link
Member

@dgieselaar dgieselaar commented Oct 31, 2019

Third time's the charm. Previous attempts:

This attempt adds two scripts in apm/scripts, that either create a tsconfig.json optimized for APM, or undo the changes in the optimization. Here's what the optimization process is like:

  • Create a tsconfig.json file in x-pack/legacy/plugins/apm
  • Rename Kibana and X-Pack tsconfig.jsons to apm.tsconfig.json so these projecst are not bootstrapped by VS Code when a file from those projects is encountered.
  • Set --skip-worktree for those tsconfig.jsons to prevent any changes to these files (including deleting them) to show up in the git staging area.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@smith
Copy link
Contributor

smith commented Oct 31, 2019

Looks great to me! I wonder if there's a benchmark we can run to compare the performance.

@dgieselaar
Copy link
Member Author

You can trace the logs from tsserver, or run a CLI check:

node x-pack/legacy/plugins/apm/scripts/optimize-tsconfig time yarn tsc --noEmit --pretty --project x-pack/tsconfig.json (55.50 real 77.30 user 3.14 sys)

vs

node x-pack/legacy/plugins/apm/scripts/optimize-tsconfig && time yarn tsc --noEmit --pretty --project x-pack/legacy/plugins/apm/tsconfig.json (real 0m22.051s
user 0m36.239s
sys 0m1.413s)

@dgieselaar dgieselaar marked this pull request as ready for review October 31, 2019 16:13
@dgieselaar dgieselaar requested a review from a team as a code owner October 31, 2019 16:13
@dgieselaar dgieselaar added apm-test-plan-7.5.0 release_note:skip Skip the PR/issue when compiling release notes v7.5.0 and removed apm-test-plan-7.5.0 v7.5.0 labels Oct 31, 2019
@sorenlouv
Copy link
Member

Maybe it’s just me but can you elaborate on the skip-worktree step in the instructions?

@sorenlouv
Copy link
Member

Oh, it looks like skip-worktree happens under the hood - not something the user is supposed to do?

Can you then document the scripts in the readme (and PR description), and how they should be invoked?

Copy link
Member

@sorenlouv sorenlouv left a comment

Choose a reason for hiding this comment

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

This looks great. Haven't played around with it much but I think you can just merge it in, and then we can take it from there.

@dgieselaar dgieselaar added v7.6.0 and removed v7.5.0 labels Nov 2, 2019
@dgieselaar
Copy link
Member Author

The optimization breaks the bootstrapping process (which uses /tsconfig.json). I'll need to think about a solution for that for a bit.

@dgieselaar dgieselaar force-pushed the apm-optimize-tsconfig branch from 0c4b867 to a5e632c Compare November 3, 2019 10:57
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@dgieselaar dgieselaar force-pushed the apm-optimize-tsconfig branch from a5e632c to 998b5af Compare November 6, 2019 11:49
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@dgieselaar dgieselaar force-pushed the apm-optimize-tsconfig branch from ba71a99 to 03753ce Compare November 6, 2019 20:40
@elasticmachine
Copy link
Contributor

💔 Build Failed

@dgieselaar dgieselaar force-pushed the apm-optimize-tsconfig branch from 03753ce to b093695 Compare November 11, 2019 11:26
@dgieselaar dgieselaar force-pushed the apm-optimize-tsconfig branch from b093695 to 42287ea Compare November 11, 2019 12:15
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@dgieselaar dgieselaar merged commit 15f2ebc into elastic:master Nov 11, 2019
@dgieselaar dgieselaar deleted the apm-optimize-tsconfig branch November 11, 2019 13:35
FrankHassanabad added a commit that referenced this pull request Mar 25, 2020
## Summary

This adds an optimization script very copied and slightly modified from:
* #49868

Usage:

Run this to do an dev tsconfig optimization:
```ts
node x-pack/legacy/plugins/siem/scripts/optimize_tsconfig
```

Run this to undo the optimization:
```ts
node x-pack/legacy/plugins/siem/scripts/unoptimize_tsconfig
```

Testing and what this does:

Run this:
```ts
node x-pack/legacy/plugins/siem/scripts/optimize_tsconfig
```

Then run your start-test-all or at least your linter, typescript check, and jest tests to make sure they all operate as expected. Restart your IDE and ensure everything works as expected.

Run `git status` and ensure it looks like no new files want to be checked in.

Open up your:
```ts
kibana/x-pack/tsconfig.json
```

And notice it is now changed when optimization has run to use a smaller set of includes.

Open up your:
```ts
kibana/tsconfig.json
```

And notice it is now changed when optimization is run to use a smaller set of includes.
FrankHassanabad added a commit to FrankHassanabad/kibana that referenced this pull request Mar 25, 2020
## Summary

This adds an optimization script very copied and slightly modified from:
* elastic#49868

Usage:

Run this to do an dev tsconfig optimization:
```ts
node x-pack/legacy/plugins/siem/scripts/optimize_tsconfig
```

Run this to undo the optimization:
```ts
node x-pack/legacy/plugins/siem/scripts/unoptimize_tsconfig
```

Testing and what this does:

Run this:
```ts
node x-pack/legacy/plugins/siem/scripts/optimize_tsconfig
```

Then run your start-test-all or at least your linter, typescript check, and jest tests to make sure they all operate as expected. Restart your IDE and ensure everything works as expected.

Run `git status` and ensure it looks like no new files want to be checked in.

Open up your:
```ts
kibana/x-pack/tsconfig.json
```

And notice it is now changed when optimization has run to use a smaller set of includes.

Open up your:
```ts
kibana/tsconfig.json
```

And notice it is now changed when optimization is run to use a smaller set of includes.
FrankHassanabad added a commit that referenced this pull request Mar 26, 2020
)

## Summary

This adds an optimization script very copied and slightly modified from:
* #49868

Usage:

Run this to do an dev tsconfig optimization:
```ts
node x-pack/legacy/plugins/siem/scripts/optimize_tsconfig
```

Run this to undo the optimization:
```ts
node x-pack/legacy/plugins/siem/scripts/unoptimize_tsconfig
```

Testing and what this does:

Run this:
```ts
node x-pack/legacy/plugins/siem/scripts/optimize_tsconfig
```

Then run your start-test-all or at least your linter, typescript check, and jest tests to make sure they all operate as expected. Restart your IDE and ensure everything works as expected.

Run `git status` and ensure it looks like no new files want to be checked in.

Open up your:
```ts
kibana/x-pack/tsconfig.json
```

And notice it is now changed when optimization has run to use a smaller set of includes.

Open up your:
```ts
kibana/tsconfig.json
```

And notice it is now changed when optimization is run to use a smaller set of includes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes v7.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants