security-dashboards-plugin icon indicating copy to clipboard operation
security-dashboards-plugin copied to clipboard

[FEATURE] [RFC] Investigate and clean up helper functions which are only used in one place

Open derek-ho opened this issue 1 year ago • 1 comments

Is your feature request related to a problem? When adding MDS support for security dashboards plugins, almost all API calls had to be changed. When investigating these changes, I noticed a few things that could be improved:

  • Some helper functions are not used at all (and may have been forgotten to be removed) - we should look into a linter rule that helps us find and remove these
  • Some helper functions are defined and only used in one place (can we move them inline)? For example https://github.com/opensearch-project/security-dashboards-plugin/blob/main/public/apps/configuration/utils/role-list-utils.tsx#L108 - is only used in one place
  • Some helper functions may be doing too simple of a job - for example httpGet helper function, which is simply extracting the get method off of the httpStart object, and calling another function with it - does this really need to exist, or can we change all calls to directly pass in the get method? Would this be more or less readable?

derek-ho avatar Apr 25 '24 18:04 derek-ho

[Triage] Hi @derek-ho thanks for filing this issue. @derek-ho to add follow up so going to leave without the label so we come back to it.

stephen-crawford avatar Apr 29 '24 15:04 stephen-crawford

I rechecked this and feel like it is fine to have a helper function used only in one place. It could reduce number of files, but maybe not worth the effort to refactor it. We can close it if people do not have a major issue with it. @scrawfor99 to ask for community feedback in the meeting right now, we can close it if no objections.

derek-ho avatar May 06 '24 15:05 derek-ho