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.

Sign up for GitHub

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

Jump to bottom

[Rules migration] Post merge feedback followups #202815

Merged
merged 4 commits into from
Dec 4, 2024
Merged

[Rules migration] Post merge feedback followups #202815

merged 4 commits into from
Dec 4, 2024

Conversation

e40pud
Copy link
Contributor

@e40pud e40pud commented Dec 3, 2024 •
edited by kibanamachine
Loading

Summary

These are the followup updated to address feedback from my previous PRs:

  • Make sure to use descriptive names specific to the siem_migrations subdomain (comment):

Make sure you use descriptive names specific to the siem_migrations subdomain. Names like RulesPage, RulesTable, useRulesColumns etc are way too generic and conflict with the rule management terminology, which would make code search more difficult.

  • Export the memo component directly everywhere (comment):

Could we export the memo component directly everywhere? It's shorter and it makes it easier to find the references in the IDE.

  • Use one hook to access APIs instead of two (comment):

I see that for every API request we have to implement 2 separate hooks. Why don't we add error handling to the same hook that does the useQuery? so we have everything in one hook. Or is there a reason to have them separate?

@e40pud e40pud added release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting Security Solution Threat Hunting Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Dec 3, 2024
@e40pud e40pud requested a review from semd December 3, 2024 21:11
@e40pud e40pud self-assigned this Dec 3, 2024
@e40pud e40pud requested a review from a team as a code owner December 3, 2024 21:11
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@elasticmachine
Copy link
Contributor

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

e40pud added 2 commits December 4, 2024 15:16
@e40pud
Review feedback
68e5554 
* use `RuleMigrationsStats`
* remove styled components and use only default ones
@e40pud
Merge branch 'main' into security/siem-migrations/post-merge-1
809a1f4
semd
semd approved these changes Dec 4, 2024
View reviewed changes
Copy link
Contributor

@semd semd left a comment

Choose a reason for hiding this comment

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

Thanks Zhenia! 💯

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 6308 6304 -4

Async chunks

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

id before after diff
securitySolution 14.6MB 14.6MB -432.0B

History

cc @e40pud

@e40pud e40pud merged commit 4d8f711 into elastic:main Dec 4, 2024
8 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12163685105

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 4, 2024
@e40pud
[Rules migration] Post merge feedback followups (elastic#202815)
bed8033 
## Summary

These are the followup updated to address feedback from my previous PRs:

* Make sure to use descriptive names specific to the `siem_migrations`
subdomain
([comment](elastic#200978 (review))):

> Make sure you use descriptive names specific to the siem_migrations
subdomain. Names like RulesPage, RulesTable, useRulesColumns etc are way
too generic and conflict with the rule management terminology, which
would make code search more difficult.

* Export the memo component directly everywhere
([comment](elastic#201597 (comment))):

> Could we export the memo component directly everywhere? It's shorter
and it makes it easier to find the references in the IDE.

* Use one hook to access APIs instead of two
([comment](elastic#202494 (comment))):

> I see that for every API request we have to implement 2 separate
hooks. Why don't we add error handling to the same hook that does the
useQuery? so we have everything in one hook. Or is there a reason to
have them separate?

(cherry picked from commit 4d8f711)
@kibanamachine kibanamachine mentioned this pull request Dec 4, 2024
Merged
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

@kibanamachine kibanamachine mentioned this pull request Dec 4, 2024
Merged
kibanamachine added a commit that referenced this pull request Dec 4, 2024
@kibanamachine @e40pud
[8.x] [Rules migration] Post merge feedback followups (#202815) (#202975
4f3b63b 
)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Rules migration] Post merge feedback followups
(#202815)](#202815)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Ievgen
Sorokopud","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-04T16:06:10Z","message":"[Rules
migration] Post merge feedback followups (#202815)\n\n##
Summary\r\n\r\nThese are the followup updated to address feedback from
my previous PRs:\r\n\r\n* Make sure to use descriptive names specific to
the
`siem_migrations`\r\nsubdomain\r\n([comment](https://github.com/elastic/kibana/pull/200978#pullrequestreview-2454582368)):\r\n\r\n>
Make sure you use descriptive names specific to the
siem_migrations\r\nsubdomain. Names like RulesPage, RulesTable,
useRulesColumns etc are way\r\ntoo generic and conflict with the rule
management terminology, which\r\nwould make code search more
difficult.\r\n\r\n\r\n* Export the memo component directly
everywhere\r\n([comment](https://github.com/elastic/kibana/pull/201597#discussion_r1858069127)):\r\n\r\n>
Could we export the memo component directly everywhere? It's
shorter\r\nand it makes it easier to find the references in the
IDE.\r\n\r\n\r\n* Use one hook to access APIs instead of
two\r\n([comment](https://github.com/elastic/kibana/pull/202494#discussion_r1867967135)):\r\n\r\n>
I see that for every API request we have to implement 2
separate\r\nhooks. Why don't we add error handling to the same hook that
does the\r\nuseQuery? so we have everything in one hook. Or is there a
reason to\r\nhave them
separate?","sha":"4d8f7111d0f8c330aab2c8347cf6d131570ff665","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:Threat
Hunting","Team: SecuritySolution","backport:prev-minor"],"title":"[Rules
migration] Post merge feedback
followups","number":202815,"url":"https://github.com/elastic/kibana/pull/202815","mergeCommit":{"message":"[Rules
migration] Post merge feedback followups (#202815)\n\n##
Summary\r\n\r\nThese are the followup updated to address feedback from
my previous PRs:\r\n\r\n* Make sure to use descriptive names specific to
the
`siem_migrations`\r\nsubdomain\r\n([comment](https://github.com/elastic/kibana/pull/200978#pullrequestreview-2454582368)):\r\n\r\n>
Make sure you use descriptive names specific to the
siem_migrations\r\nsubdomain. Names like RulesPage, RulesTable,
useRulesColumns etc are way\r\ntoo generic and conflict with the rule
management terminology, which\r\nwould make code search more
difficult.\r\n\r\n\r\n* Export the memo component directly
everywhere\r\n([comment](https://github.com/elastic/kibana/pull/201597#discussion_r1858069127)):\r\n\r\n>
Could we export the memo component directly everywhere? It's
shorter\r\nand it makes it easier to find the references in the
IDE.\r\n\r\n\r\n* Use one hook to access APIs instead of
two\r\n([comment](https://github.com/elastic/kibana/pull/202494#discussion_r1867967135)):\r\n\r\n>
I see that for every API request we have to implement 2
separate\r\nhooks. Why don't we add error handling to the same hook that
does the\r\nuseQuery? so we have everything in one hook. Or is there a
reason to\r\nhave them
separate?","sha":"4d8f7111d0f8c330aab2c8347cf6d131570ff665"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/202815","number":202815,"mergeCommit":{"message":"[Rules
migration] Post merge feedback followups (#202815)\n\n##
Summary\r\n\r\nThese are the followup updated to address feedback from
my previous PRs:\r\n\r\n* Make sure to use descriptive names specific to
the
`siem_migrations`\r\nsubdomain\r\n([comment](https://github.com/elastic/kibana/pull/200978#pullrequestreview-2454582368)):\r\n\r\n>
Make sure you use descriptive names specific to the
siem_migrations\r\nsubdomain. Names like RulesPage, RulesTable,
useRulesColumns etc are way\r\ntoo generic and conflict with the rule
management terminology, which\r\nwould make code search more
difficult.\r\n\r\n\r\n* Export the memo component directly
everywhere\r\n([comment](https://github.com/elastic/kibana/pull/201597#discussion_r1858069127)):\r\n\r\n>
Could we export the memo component directly everywhere? It's
shorter\r\nand it makes it easier to find the references in the
IDE.\r\n\r\n\r\n* Use one hook to access APIs instead of
two\r\n([comment](https://github.com/elastic/kibana/pull/202494#discussion_r1867967135)):\r\n\r\n>
I see that for every API request we have to implement 2
separate\r\nhooks. Why don't we add error handling to the same hook that
does the\r\nuseQuery? so we have everything in one hook. Or is there a
reason to\r\nhave them
separate?","sha":"4d8f7111d0f8c330aab2c8347cf6d131570ff665"}}]}]
BACKPORT-->

Co-authored-by: Ievgen Sorokopud <[email protected]>
SoniaSanzV pushed a commit to SoniaSanzV/kibana that referenced this pull request Dec 9, 2024
@e40pud @SoniaSanzV
[Rules migration] Post merge feedback followups (elastic#202815)
0ed26d3 
## Summary

These are the followup updated to address feedback from my previous PRs:

* Make sure to use descriptive names specific to the `siem_migrations`
subdomain
([comment](elastic#200978 (review))):

> Make sure you use descriptive names specific to the siem_migrations
subdomain. Names like RulesPage, RulesTable, useRulesColumns etc are way
too generic and conflict with the rule management terminology, which
would make code search more difficult.


* Export the memo component directly everywhere
([comment](elastic#201597 (comment))):

> Could we export the memo component directly everywhere? It's shorter
and it makes it easier to find the references in the IDE.


* Use one hook to access APIs instead of two
([comment](elastic#202494 (comment))):

> I see that for every API request we have to implement 2 separate
hooks. Why don't we add error handling to the same hook that does the
useQuery? so we have everything in one hook. Or is there a reason to
have them separate?
SoniaSanzV pushed a commit to SoniaSanzV/kibana that referenced this pull request Dec 9, 2024
@e40pud @SoniaSanzV
[Rules migration] Post merge feedback followups (elastic#202815)
2925c54 
## Summary

These are the followup updated to address feedback from my previous PRs:

* Make sure to use descriptive names specific to the `siem_migrations`
subdomain
([comment](elastic#200978 (review))):

> Make sure you use descriptive names specific to the siem_migrations
subdomain. Names like RulesPage, RulesTable, useRulesColumns etc are way
too generic and conflict with the rule management terminology, which
would make code search more difficult.


* Export the memo component directly everywhere
([comment](elastic#201597 (comment))):

> Could we export the memo component directly everywhere? It's shorter
and it makes it easier to find the references in the IDE.


* Use one hook to access APIs instead of two
([comment](elastic#202494 (comment))):

> I see that for every API request we have to implement 2 separate
hooks. Why don't we add error handling to the same hook that does the
useQuery? so we have everything in one hook. Or is there a reason to
have them separate?
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 9, 2024
@e40pud @CAWilson94
[Rules migration] Post merge feedback followups (elastic#202815)
4b0c1d5 
## Summary

These are the followup updated to address feedback from my previous PRs:

* Make sure to use descriptive names specific to the `siem_migrations`
subdomain
([comment](elastic#200978 (review))):

> Make sure you use descriptive names specific to the siem_migrations
subdomain. Names like RulesPage, RulesTable, useRulesColumns etc are way
too generic and conflict with the rule management terminology, which
would make code search more difficult.


* Export the memo component directly everywhere
([comment](elastic#201597 (comment))):

> Could we export the memo component directly everywhere? It's shorter
and it makes it easier to find the references in the IDE.


* Use one hook to access APIs instead of two
([comment](elastic#202494 (comment))):

> I see that for every API request we have to implement 2 separate
hooks. Why don't we add error handling to the same hook that does the
useQuery? so we have everything in one hook. Or is there a reason to
have them separate?
Samiul-TheSoccerFan pushed a commit to Samiul-TheSoccerFan/kibana that referenced this pull request Dec 10, 2024
@e40pud @Samiul-TheSoccerFan
[Rules migration] Post merge feedback followups (elastic#202815)
253d519 
## Summary

These are the followup updated to address feedback from my previous PRs:

* Make sure to use descriptive names specific to the `siem_migrations`
subdomain
([comment](elastic#200978 (review))):

> Make sure you use descriptive names specific to the siem_migrations
subdomain. Names like RulesPage, RulesTable, useRulesColumns etc are way
too generic and conflict with the rule management terminology, which
would make code search more difficult.


* Export the memo component directly everywhere
([comment](elastic#201597 (comment))):

> Could we export the memo component directly everywhere? It's shorter
and it makes it easier to find the references in the IDE.


* Use one hook to access APIs instead of two
([comment](elastic#202494 (comment))):

> I see that for every API request we have to implement 2 separate
hooks. Why don't we add error handling to the same hook that does the
useQuery? so we have everything in one hook. Or is there a reason to
have them separate?
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 12, 2024
@e40pud @CAWilson94
[Rules migration] Post merge feedback followups (elastic#202815)
fae31f2 
## Summary

These are the followup updated to address feedback from my previous PRs:

* Make sure to use descriptive names specific to the `siem_migrations`
subdomain
([comment](elastic#200978 (review))):

> Make sure you use descriptive names specific to the siem_migrations
subdomain. Names like RulesPage, RulesTable, useRulesColumns etc are way
too generic and conflict with the rule management terminology, which
would make code search more difficult.


* Export the memo component directly everywhere
([comment](elastic#201597 (comment))):

> Could we export the memo component directly everywhere? It's shorter
and it makes it easier to find the references in the IDE.


* Use one hook to access APIs instead of two
([comment](elastic#202494 (comment))):

> I see that for every API request we have to implement 2 separate
hooks. Why don't we add error handling to the same hook that does the
useQuery? so we have everything in one hook. Or is there a reason to
have them separate?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@semd semd semd approved these changes

Assignees

@e40pud e40pud

Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting Security Solution Threat Hunting Team v8.18.0 v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@e40pud @elasticmachine @kibanamachine @semd