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] Convert camelCase to snake_case for all files and folders #107347

Closed
2 tasks done
sorenlouv opened this issue Aug 1, 2021 · 7 comments · Fixed by #123648
Closed
2 tasks done

[APM] Convert camelCase to snake_case for all files and folders #107347

sorenlouv opened this issue Aug 1, 2021 · 7 comments · Fixed by #123648
Assignees
Labels
good first issue low hanging fruit Team:APM All issues that need APM UI Team support technical debt Improvement of the software architecture and operational architecture

Comments

@sorenlouv
Copy link
Member

sorenlouv commented Aug 1, 2021

Kibana is using snake_case for all files and folders but APM has a mixed filenaming scheme. APM should use snake_case like the rest of Kibana.
APM has disabled Kibana's filename check. This check should be re-enabled again.

Steps

  1. Remove/uncomment the following lines to re-add case-checking of APM files:

    // TODO fix file names in APM to remove these
    'x-pack/plugins/apm/public/**/*',
    'x-pack/plugins/apm/scripts/**/*',
    'x-pack/plugins/apm/e2e/**/*',

  2. Run node scripts/check_file_casing.js from kibana root

  3. Fix all the filename errors listed in the step above

  4. Run node scripts/check_file_casing.js again to verify all errors are fixed

Notes
If you're on a platform with a case-insensitive filesystem (macOS by default), you'll need to have

[core]
	ignorecase = false

in .git/config.

If you're just changing the casing of a filename (from upper to lowercase) you may need to use git mv -f to rename the files and make sure github notices.

Tasks

  • x-pack/plugins/apm/public/**/*
  • x-pack/plugins/apm/scripts/**/*
@sorenlouv sorenlouv added [zube]: Backlog good first issue low hanging fruit low hanging fruit DO NOT USE. Use `good first issue` instead Team:APM All issues that need APM UI Team support labels Aug 1, 2021
@elasticmachine
Copy link
Contributor

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

@smith smith self-assigned this Aug 3, 2021
@smith smith added technical debt Improvement of the software architecture and operational architecture [zube]: Backlog and removed low hanging fruit DO NOT USE. Use `good first issue` instead [zube]: In Progress labels Aug 3, 2021
@smith smith removed their assignment Aug 3, 2021
@mohitsaxenaknoldus
Copy link

mohitsaxenaknoldus commented Aug 4, 2021

Please assign it to me, I'll do it. Also, tell me which files and folders you want me to include.

@smith
Copy link
Contributor

smith commented Aug 5, 2021

@mohitsaxenaknoldus if you delete these lines: https://github.com/smith/kibana/blob/7216bab3521bd395a019ef8af3392d25f0f912a6/src/dev/precommit_hook/casing_check_config.js#L60-L64

Then run node scripts/check_file_casing.js from the kibana root, all the filenames it outputs in x-pack/plugins/apm are the ones that need changed.

If you're on a platform with a case-insensitive filesystem (macOS by default), you'll need to have

[core]
	ignorecase = false

in .git/config.

If you're just changing the casing of a filename (from upper to lowercase) you may need to use git mv -f to rename the files and make sure github notices.

I've tried doing this with some automated tools, but there might be a lot of work that has to be done manually.

If you're doing this work you'll want to be fetching from upstream (elastic/kibana) and merging from upstream frequently as code is changing quite often in Kibana and this kind of change can have a lot of conflicts if not kept up to date.

Please let me know if you have any problems or questions.

@sorenlouv
Copy link
Member Author

sorenlouv commented Aug 5, 2021

Thanks @smith! I've added your comment to the issue description.

@mohitsaxenaknoldus
Copy link

When I do: node scripts/check_file_casing.js Its giving me this:

Error: Cannot find module '@kbn/optimizer'
Require stack:
- /home/knoldus/OS/kibana/src/setup_node_env/index.js
- /home/knoldus/OS/kibana/scripts/check_file_casing.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:902:15)
    at Module.Hook._require.Module.require (/home/knoldus/OS/kibana/node_modules/require-in-the-middle/index.js:61:29)
    at require (internal/modules/cjs/helpers.js:92:18)
    at Object.<anonymous> (/home/knoldus/OS/kibana/src/setup_node_env/index.js:11:1)
    at Module._compile (internal/modules/cjs/loader.js:1085:14)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1114:10)
    at Module.load (internal/modules/cjs/loader.js:950:32)
    at Function.Module._load (internal/modules/cjs/loader.js:790:14)
    at Module.require (internal/modules/cjs/loader.js:974:19)
    at require (internal/modules/cjs/helpers.js:92:18)

@smith
Copy link
Contributor

smith commented Aug 5, 2021

@mohitsaxenaknoldus you probably need to run yarn kbn bootstrap to make sure all dependencies are installed and set up. See https://www.elastic.co/guide/en/kibana/master/development-getting-started.html for more.

@kpatticha
Copy link
Contributor

I'm working on renaming all the files in x-pack/plugins/apm/public/**/* folder.
Draft PR to track the progress #119899

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue low hanging fruit Team:APM All issues that need APM UI Team support technical debt Improvement of the software architecture and operational architecture
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants