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

Rename PipelineProcessorsEditor to PipelineEditor to shorten import path to a length that Windows can handle #89645

Merged
merged 2 commits into from
Jan 29, 2021

Conversation

cjcenizal
Copy link
Contributor

This change is intended to address #89505 (comment), in which the long names of our folders and files results in a path length error on Windows:

info [  kibana  ] Checking Windows for paths > 200 characters
02:58:11     │ERROR failure 3 sec
02:58:11     │ERROR Error: Windows has a path limit of 260 characters so we limit the length of paths in Kibana to 200 characters  and the following files exceed this limit:
02:58:11     │       - x-pack/plugins/ingest_pipelines/target/types/public/application/components/pipeline_processors_editor/components/pipeline_processors_editor_item_tooltip/pipeline_processors_editor_item_tooltip.d.ts.map

This change also disambiguates this component from a child component of the same name.

…ath to a length that Windows can handle, and to disambiguate with child component of the same name.
@cjcenizal cjcenizal added chore v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more release_note:skip Skip the PR/issue when compiling release notes Feature:Ingest Node Pipelines Ingest node pipelines management v7.12.0 labels Jan 28, 2021
@cjcenizal cjcenizal requested review from a team as code owners January 28, 2021 21:44
@elasticmachine
Copy link
Contributor

Pinging @elastic/es-ui (Team:Elasticsearch UI)

Copy link
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

This works locally after pulling the changes into #89505.
I'll wait for a merge to reflect the changes in the ingest_pipelines migration to a TS project PR.

Copy link
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

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

Changes look good to me @cjcenizal, I would like to add two items to this chore for consistency in naming. We should rename the following files + components too:

  • x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/pipeline_processors_editor_item
  • x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/pipeline_processors_editor_item_tooltip

I don't view these as blocking for this PR, but I think we should have a chore issue to address them if they if they are not going to be done in this PR.

@jloleysens
Copy link
Contributor

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
ingestPipelines 723.6KB 723.3KB -305.0B

History

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

@cjcenizal
Copy link
Contributor Author

Thanks @jloleysens! I wasn't sure about those two, since there is a pipeline_processors_editor.tsx component in the root of the parent directory shared by the two components you referenced. Do those components have more affinity with the pipeline_editor.tsx component I renamed, or the pipeline_processors_editor.tsx component? I'll merge this PR for now, and if you think they share more affinity with pipeline_editor.tsx then I'll proceed with your suggestion.

@cjcenizal cjcenizal merged commit 9286b13 into elastic:master Jan 29, 2021
@cjcenizal cjcenizal deleted the chore/ingest-pipeline-path-length branch January 29, 2021 17:21
cjcenizal added a commit that referenced this pull request Jan 29, 2021
…ath to a length that Windows can handle, and to disambiguate with child component of the same name. (#89645) (#89757)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Feature:Ingest Node Pipelines Ingest node pipelines management release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.12.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants