[APM] chore: Unify naming of 'apm/scripts/**/*' with snake_case #118328

kpatticha · 2021-11-11T12:35:28Z

Summary

Rename all files and folders inside apm/scripts in order to follow snake_case naming convention.
Addresses part of #107347

elasticmachine · 2021-11-11T12:35:32Z

Pinging @elastic/apm-ui (Team:apm)

sorenlouv

Thanks for doing this. I think it was intention to make cli commands (scripts) kebab-case rather than snake_case.
@elastic/apm-ui any opinions here?

kpatticha · 2021-11-11T16:18:51Z

Thanks for doing this. I think it was intention to make cli commands (scripts) kebab-case rather than snake_case. @elastic/apm-ui any opinions here?

thanks good to know :D

After running ls -Rl in main branch, I've noticed that the files located in the root of /scripts (the ones that run as cli) are kebab-case, which make sense to me. However, there are some inconsistencies in some folders, ex shared

/scripts/

total 72
drwxr-xr-x  3 kpatticha  staff    96 Nov 11 16:54 aggregate-latency-metrics
-rw-r--r--  1 kpatticha  staff   678 Nov 11 16:54 aggregate-latency-metrics.js
drwxr-xr-x  6 kpatticha  staff   192 Nov 11 16:54 create-apm-users-and-roles
-rw-r--r--  1 kpatticha  staff   911 Nov 11 16:54 create-apm-users-and-roles.js
drwxr-xr-x  3 kpatticha  staff    96 Nov 11 16:54 create-functional-tests-archive
-rw-r--r--  1 kpatticha  staff   464 Nov 11 16:54 create-functional-tests-archive.js
-rw-r--r--  1 kpatticha  staff   992 Nov  8 23:39 eslint.js
drwxr-xr-x  7 kpatticha  staff   224 Nov 11 16:54 optimize-tsconfig
-rw-r--r--  1 kpatticha  staff   406 Nov 11 16:54 optimize-tsconfig.js
-rw-r--r--  1 kpatticha  staff    92 Nov  2 17:44 package.json
-rw-r--r--  1 kpatticha  staff  2473 Nov 11 16:54 precommit.js
drwxr-xr-x  8 kpatticha  staff   256 Nov 11 16:54 shared
drwxr-xr-x  6 kpatticha  staff   192 Nov  5 15:45 test
-rw-r--r--  1 kpatticha  staff   412 Nov 11 16:54 unoptimize-tsconfig.js
drwxr-xr-x  4 kpatticha  staff   128 Nov 11 16:54 upload-telemetry-data
-rw-r--r--  1 kpatticha  staff   454 Nov 11 16:54 upload-telemetry-data.js

/scripts/shared

./shared:
total 48
drwxr-xr-x   8 kpatticha  staff   256 Nov 11 16:54 .
drwxr-xr-x  19 kpatticha  staff   608 Nov 11 16:54 ..
-rw-r--r--   1 kpatticha  staff  1598 Nov 11 16:54 create-or-update-index.ts
-rw-r--r--   1 kpatticha  staff   996 Nov 11 16:54 download-telemetry-template.ts
-rw-r--r--   1 kpatticha  staff   620 Nov 11 16:54 get-http-auth.ts
-rw-r--r--   1 kpatticha  staff  1439 Nov  5 10:36 get_es_client.ts
-rw-r--r--   1 kpatticha  staff   521 Nov  2 17:44 parse_index_url.ts
-rw-r--r--   1 kpatticha  staff  1679 Nov 11 16:54 read-kibana-config.ts

I don't have a strong opinion on that, i'm always in favour of consistency and conventions that work for the team.
For example, it can be kebab-case for files that we run in the command line and the rest could be snake_case or we can convert all to snake_case and follow kibana's convention. I'd like to hear other ideas. @elastic/apm-ui

sorenlouv · 2021-11-11T19:03:33Z

For example, it can be kebab-case for files that we run in the command line and the rest could be snake_case or we can convert all to snake_case and follow kibana's convention. I'd like to hear other ideas

Sounds good to me. I don't have any strong opinions as long as we can find consensus on the team

kpatticha · 2021-11-15T09:57:33Z

@elasticmachine merge upstream

kibana-ci · 2021-11-15T11:25:04Z

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

💚 Build #6175 succeeded ffe531e

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

sorenlouv

I noticed that kibana core also uses snake_case for scripts:

https://github.com/elastic/kibana/tree/1a321bce77393810216685ae0841b74588bf3be3/scripts

…igrate-away-from-injected-css-js * 'main' of github.com:elastic/kibana: (221 commits) [Reporting] Add log level to config (elastic#118319) [Metrics UI] Skip failing waffle chart color palette test (elastic#118527) [APM] chore: Unify naming of 'apm/scripts/**/*' with snake_case (elastic#118328) skip flaky suites (elastic#99581, elastic#118356, elastic#118474) [Alerting] Initial implementation of alerting task `cancel()` (elastic#114289) chore(NA): creates pkg_npm_types bazel rule (elastic#116465) skip flaky suite (elastic#116892) Bump chromedriver to 95.0.0 (elastic#116724) [Data visualizer] Improve design of expanded rows (elastic#118125) [APM] prefer ECS field names for HTTP and URL (elastic#118485) Update query_debugging_in_development_and_production.md (elastic#118491) [Uptime] adjust Elastic Synthetics integration functional tests (elastic#118163) [kbn/rule-data-utils] add submodules and require public use them (elastic#117963) [DOCS] Refresh APM correlation screenshots (elastic#116723) (elastic#118577) Handles ns to ms conversion for event loop delay metrics (elastic#118447) [Cases] Rename functional tests folder (elastic#118490) dummy commit skip flaky suite (elastic#118593) Improve workpad schema validation (elastic#97838) skip flaky suite (elastic#118584) ... # Conflicts: # src/plugins/dashboard/public/application/embeddable/viewport/dashboard_viewport.tsx

kibanamachine · 2021-11-18T05:34:59Z

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 118328 or prevent reminders by adding the backport:skip label.

kibanamachine · 2021-11-18T14:49:18Z

The following labels were identified as gaps in your version labels and will be added automatically:

v8.1.0

If any of these should not be on your pull request, please manually remove them.

…tic#118328) * [APM] chore: Unify naming of 'apm/scripts/**/*' with snake_case * Fix wrong path Co-authored-by: Kibana Machine <[email protected]>

kibanamachine · 2021-11-18T14:52:20Z

💚 Backport successful

Status	Branch	Result
✅	8.0

This backport PR will be merged automatically after passing CI.

) (#119040) * [APM] chore: Unify naming of 'apm/scripts/**/*' with snake_case * Fix wrong path Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kate Patticha <[email protected]>

…tic#118328) * [APM] chore: Unify naming of 'apm/scripts/**/*' with snake_case * Fix wrong path Co-authored-by: Kibana Machine <[email protected]>

) * [APM] chore: Unify naming of 'apm/scripts/**/*' with snake_case * Fix wrong path Co-authored-by: Kibana Machine <[email protected]>

[APM] chore: Unify naming of 'apm/scripts/**/*' with snake_case

4319c94

kpatticha added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes labels Nov 11, 2021

kpatticha requested review from a team as code owners November 11, 2021 12:35

botelastic bot added the Team:APM All issues that need APM UI Team support label Nov 11, 2021

Fix wrong path

ffe531e

sorenlouv reviewed Nov 11, 2021

View reviewed changes

jbudz approved these changes Nov 11, 2021

View reviewed changes

Merge branch 'main' into unify-scripts-naming

3f60860

elastic deleted a comment from kibanamachine Nov 15, 2021

smith approved these changes Nov 15, 2021

View reviewed changes

sorenlouv approved these changes Nov 15, 2021

View reviewed changes

kpatticha merged commit e56627a into elastic:main Nov 16, 2021

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Nov 18, 2021

sorenlouv added the auto-backport Deprecated - use backport:version if exact versions are needed label Nov 18, 2021

kibanamachine added the v8.1.0 label Nov 18, 2021

kibanamachine mentioned this pull request Nov 18, 2021

[8.0] [APM] chore: Unify naming of 'apm/scripts/**/*' with snake_case (#118328) #119040

Merged

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Nov 18, 2021

dmlemeshko pushed a commit that referenced this pull request Nov 29, 2021

[APM] chore: Unify naming of 'apm/scripts/**/*' with snake_case (#118328

2cade33

) * [APM] chore: Unify naming of 'apm/scripts/**/*' with snake_case * Fix wrong path Co-authored-by: Kibana Machine <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[APM] chore: Unify naming of 'apm/scripts/**/*' with snake_case #118328

[APM] chore: Unify naming of 'apm/scripts/**/*' with snake_case #118328

kpatticha commented Nov 11, 2021

elasticmachine commented Nov 11, 2021

sorenlouv left a comment

kpatticha commented Nov 11, 2021

sorenlouv commented Nov 11, 2021 •

edited

Loading

kpatticha commented Nov 15, 2021

kibana-ci commented Nov 15, 2021

sorenlouv left a comment

kibanamachine commented Nov 18, 2021

kibanamachine commented Nov 18, 2021

kibanamachine commented Nov 18, 2021

[APM] chore: Unify naming of 'apm/scripts/**/*' with snake_case #118328

[APM] chore: Unify naming of 'apm/scripts/**/*' with snake_case #118328

Conversation

kpatticha commented Nov 11, 2021

Summary

elasticmachine commented Nov 11, 2021

sorenlouv left a comment

Choose a reason for hiding this comment

kpatticha commented Nov 11, 2021

sorenlouv commented Nov 11, 2021 • edited Loading

kpatticha commented Nov 15, 2021

kibana-ci commented Nov 15, 2021

💚 Build Succeeded

Metrics [docs]

History

sorenlouv left a comment

Choose a reason for hiding this comment

kibanamachine commented Nov 18, 2021

kibanamachine commented Nov 18, 2021

kibanamachine commented Nov 18, 2021

💚 Backport successful

sorenlouv commented Nov 11, 2021 •

edited

Loading