Skip to content

[nit][pre-req] Split EPM Home into components#114431

Merged
clintandrewhall merged 5 commits intoelastic:masterfrom
clintandrewhall:fleet/pre-req-home-split
Oct 12, 2021
Merged

[nit][pre-req] Split EPM Home into components#114431
clintandrewhall merged 5 commits intoelastic:masterfrom
clintandrewhall:fleet/pre-req-home-split

Conversation

@clintandrewhall
Copy link
Copy Markdown
Contributor

@clintandrewhall clintandrewhall commented Oct 11, 2021 •
edited
Loading

Summary

To make changes like #114432 easier to understand, I split the EPM Home Page into is disparate components. There are also some nominal performance tweaks, (like moving static i18n calls out of the render cycle to module-level consts, etc).

This makes the next PR easier to grok, but also makes the individual portions of the page easier to test.

Tech debt: split the new components into "connected" and "pure" components to make testing easier.

@clintandrewhall clintandrewhall added review loe:small Small Level of Effort v8.0.0 release_note:skip Skip the PR/issue when compiling release notes impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Team:Fleet Team label for Observability Data Collection Fleet team auto-backport Deprecated - use backport:version if exact versions are needed v7.16.0 Feature:Unified Integrations Unified Integrations view feature labels Oct 11, 2021
@clintandrewhall clintandrewhall requested a review from a team as a code owner October 11, 2021 04:54
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Oct 11, 2021

Pinging @elastic/fleet (Team:Fleet)

@clintandrewhall clintandrewhall mentioned this pull request Oct 11, 2021
Merged
@clintandrewhall
Copy link
Copy Markdown
Contributor Author

clintandrewhall commented Oct 11, 2021

@elasticmachine merge upstream

thomasneirynck
thomasneirynck reviewed Oct 11, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@thomasneirynck thomasneirynck left a comment

Choose a reason for hiding this comment

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

OK for shipper change, but I would remove the change to categories. IntegrationCategory was already strongly typed, and this change adds a duplication of data-structures (basically, we would now require two lists now, once as an array and once as a map).

thomasneirynck
thomasneirynck approved these changes Oct 11, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@thomasneirynck thomasneirynck left a comment

Choose a reason for hiding this comment

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

fwiw - I'd unhook this change from that other strong-typing-for-shipper PR, it introduces a chain of dependencies, and they could be handled in parallel.

This will also create merge conflicts with #114429

@clintandrewhall clintandrewhall enabled auto-merge (squash) October 12, 2021 02:42
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Oct 12, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
fleet 576 578 +2

Async chunks

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

id before after diff
fleet 625.7KB 625.7KB +9.0B

History

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

@clintandrewhall clintandrewhall merged commit ab5ecc4 into elastic:master Oct 12, 2021
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Oct 12, 2021

💔 Backport failed

Status Branch Result
❌ 7.x Commit could not be cherrypicked due to conflicts

To backport manually run:
node scripts/backport --pr 114431

This was referenced Oct 12, 2021
Merged
Merged
thomasneirynck pushed a commit to thomasneirynck/kibana that referenced this pull request Oct 13, 2021
@clintandrewhall @kibanamachine @thomasneirynck
[nit][pre-req] Split EPM Home into components (elastic#114431)
04fa293 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@thomasneirynck
Copy link
Copy Markdown
Contributor

thomasneirynck commented Oct 13, 2021

New backport here #114828

thomasneirynck added a commit that referenced this pull request Oct 13, 2021
@thomasneirynck @clintandrewhall @kibanamachine
[nit][pre-req] Split EPM Home into components (#114431) (#114828)
e6e60c1 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Clint Andrew Hall <clint.hall@elastic.co>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@clintandrewhall clintandrewhall deleted the fleet/pre-req-home-split branch December 29, 2021 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@joshdover joshdover joshdover approved these changes

@thomasneirynck thomasneirynck thomasneirynck approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

auto-backport Deprecated - use backport:version if exact versions are needed Feature:Unified Integrations Unified Integrations view feature impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:small Small Level of Effort release_note:skip Skip the PR/issue when compiling release notes review Team:Fleet Team label for Observability Data Collection Fleet team v7.16.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@clintandrewhall @elasticmachine @kibanamachine @thomasneirynck @joshdover