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

[Security Solution][DQD] Tech Debt Cleanup #190964

Closed
kapral18 opened this issue Aug 21, 2024 · 2 comments
Closed

[Security Solution][DQD] Tech Debt Cleanup #190964

kapral18 opened this issue Aug 21, 2024 · 2 comments
Assignees
Labels
8.16 candidate Team:Threat Hunting:Explore Team:Threat Hunting Security Solution Threat Hunting Team technical debt Improvement of the software architecture and operational architecture

Comments

@kapral18
Copy link
Contributor

kapral18 commented Aug 21, 2024

  • cleanup dead code
  • flatten scaffold where possible
  • restructure react components nesting so that used dependencies are colocated
  • DRY code up where it makes sense
@kapral18 kapral18 self-assigned this Aug 21, 2024
@botelastic botelastic bot added the needs-team Issues missing a team label label Aug 21, 2024
@kapral18 kapral18 added Team:Threat Hunting Security Solution Threat Hunting Team Team:Threat Hunting:Explore and removed needs-team Issues missing a team label labels Aug 21, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting-explore (Team:Threat Hunting:Explore)

@kapral18 kapral18 added the technical debt Improvement of the software architecture and operational architecture label Aug 21, 2024
kapral18 added a commit to kapral18/kibana that referenced this issue Aug 26, 2024
addresses elastic#190964

First in the series of PRs to address general DQD tech debt

This one flattens and makes structure a little more logical

- rename top level data_quality/ folder to data_quality_panel/ in line
  with the actual component name in index.tsx
- remove redundant nested data_quality_panel/ folder
kapral18 added a commit that referenced this issue Aug 26, 2024
addresses #190964

Second in the series of PRs to address general DQD tech debt. Currently
this PR build on the work of
#190970.

So this PR restructures components to colocate component throughout the
hierarchy.

So instead of:

```bash
root/
  component1/
  childOfComponent1/
  grandChildOfComponent1/
  component2/
  use_hook_used_for_1/
  use_hook_used_for_2/
```

we use:

```bash
root/
  component1/
    hooks/
      use_hook_used_for_1/
    childOfComponent1/
      grandChildOfComponent1/
  component2/
    hooks/
      use_hook_used_for_2/
```

PROs of such scaffold:
- complete and clear hierarchical visibility into component structure of
the entire DQD codebase
- ability to easily introduce and integrate a new change and calculate
its impact on the tree of components
- ability to easily remove colocated functionality without having to
scout through the convoluted DQD code
- clear understanding of where shared code should live as opposed to
know when its shoved into top level by default with other non shared
code
- since nesting too deep has an import name readability tax it forces us
to think about not splitting our components into too many small parts
but rather keep it balanced, as opposed to now where flat structure
incentivizes free and cheap fragmentation as seen with component like
<body />.

CONS:

- import names have too many `../../../../../../../../../`. It is
fixable by ts paths/webpack aliases, but on the other hand especially if
there are many of those it's an indication of potential architectural
smell, that needs to be addressed (which is a PRO).

Imho, overall visibility trumps any cons and facilitates greater ease of
adding new and changing existing functionality with more confidence.

## Before

![image](https://github.com/user-attachments/assets/89062883-c40a-410d-af43-8dbe3e712475)

## After

![image](https://github.com/user-attachments/assets/83e33a85-cf3e-4cb1-a56d-c7f4f27a1f37)
kapral18 added a commit that referenced this issue Aug 26, 2024
addresses #190964

Third in the series of PRs to address general DQD tech debt

This one builds on previous 2 PRs 

#190970
#190978 

Gist of changes:

- split top level helpers into series of utils/* files
- each utils/ file is named after common behavior it export or works
with.
- cleanup dead code
kapral18 added a commit that referenced this issue Aug 28, 2024
…1245)

addresses #190964

Fourth in the series of PRs to address general DQD tech debt

This one builds on previous 3 PRs

#190970
#190978
#191233

Gist of changes:

split lower level helpers into series of utils/* files
each utils/ file is named after common behavior it export or works with.
cleanup dead code
kapral18 added a commit that referenced this issue Aug 28, 2024
#191264)

addresses #190964

Fifth in the series of PRs to address general DQD tech debt

This one builds on previous 4 PRs

#190970
#190978
#191233
#191245

Gist of changes:

- split gigantic markdown helper file and colocate the parts where they
belong
- dedupe translations
- cleanup dead code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.16 candidate Team:Threat Hunting:Explore Team:Threat Hunting Security Solution Threat Hunting Team technical debt Improvement of the software architecture and operational architecture
Projects
None yet
Development

No branches or pull requests

2 participants