Skip to content

Remove unused package#158597

Merged
oatkiller merged 2 commits intoelastic:mainfrom
oatkiller:remove-unused-marge-package
Jun 12, 2023
Merged

Remove unused package#158597
oatkiller merged 2 commits intoelastic:mainfrom
oatkiller:remove-unused-marge-package

Conversation

@oatkiller
Copy link
Copy Markdown
Contributor

@oatkiller oatkiller commented May 26, 2023 •
edited
Loading

Summary

The marge package is included in package.json, but it doesn't appear to be used.

note: there is a marge binary, but that is provided by https://npm.io/package/@danielkalen/mochawesome-report-generator and is unrelated.

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk Probability Severity Mitigation/Notes
The package is actually used Low Unknown Revert the PR is this happens.

For maintainers

@oatkiller oatkiller added the release_note:skip Skip the PR/issue when compiling release notes label May 26, 2023
@oatkiller oatkiller requested a review from mistic May 26, 2023 18:41
@oatkiller oatkiller marked this pull request as ready for review May 26, 2023 18:41
@kibana-ci
Copy link
Copy Markdown

kibana-ci commented May 26, 2023

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
securitySolution 400 404 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
securitySolution 480 484 +4
total +6

History

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

@oatkiller oatkiller enabled auto-merge (squash) May 30, 2023 16:58
mistic
mistic approved these changes Jun 12, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@mistic mistic left a comment

Choose a reason for hiding this comment

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

lgtm

@oatkiller oatkiller merged commit ab29f08 into elastic:main Jun 12, 2023
@mistic mistic added the chore label Jun 12, 2023
@kibanamachine kibanamachine added v8.9.0 backport:skip This PR does not require backporting labels Jun 12, 2023
mistic added a commit that referenced this pull request Jun 12, 2023
@mistic
Revert "Remove unused package (#158597)"
3164006 
This reverts commit ab29f08.
@mistic
Copy link
Copy Markdown
Contributor

mistic commented Jun 12, 2023

Actually I had to revert this in ab29f08 as there is direct invokations to this module into some packages tasks like https://github.com/elastic/kibana/blob/main/x-pack/plugins/fleet/package.json#L14

@oatkiller
Copy link
Copy Markdown
Contributor Author

oatkiller commented Jun 12, 2023

@mistic The marge binary should be coming from a different package. See the ls -la for the binary:


oatkiller@oatkillers-mbp kibana % ls -la node_modules/.bin/marge
lrwxr-xr-x  1 oatkiller  staff  42 Jun  7 12:45 node_modules/.bin/marge -> ../mochawesome-report-generator/bin/cli.js

Here are the docs: https://www.npmjs.com/package/mochawesome-report-generator

Did removing this cause an issue?

@mistic
Copy link
Copy Markdown
Contributor

mistic commented Jun 12, 2023

@oatkiller the chain of deps looks like mochawesome-merge > mochawesome-report-generator > marge . Are we sure we will only need marge as long as we keep needing mochawesome-merge ? It looks strange to directly invoke usage on a dep we don't explicitly install but you probably have more knowledge on that part of the code base than me. If you think my previous question will always hold true, please reopen the PR to remerge this again

jloleysens added a commit to jloleysens/kibana that referenced this pull request Jun 13, 2023
@jloleysens
Merge branch 'main' into do-not-do-version-check-on-serverless
a67fd82 
* main: (199 commits)
  [Lens] Add custom formatter within the Lens editor (elastic#158468)
  [Uptime] Hide app if no data is available (elastic#159118)
  [CodeEditor] Add grok highlighting (elastic#159334)
  Expose decoded cloudId components from the cloud plugin's contract (elastic#159442)
  [Profiling] Use collector and symbolizer integrations in the add data page (elastic#159481)
  [Infrastructure UI] Hosts View: Unified Search bar with auto-refresh enabled (elastic#157011)
  [APM] Add feature flag for not available apm schema (elastic#158911)
  [Lens] Remove deprecated componentWillReceiveProps usage (elastic#159502)
  [api-docs] 2023-06-13 Daily api_docs build (elastic#159536)
  [data views] Use versioned router for REST routes (elastic#158608)
  [maps] fix geo line source not loaded unless maps application is opened (elastic#159432)
  [Enterprise Search][Search application]Fix Create Api key url (elastic#159519)
  [Security Solution] Increase timeout for indexing hosts (elastic#159518)
  dashboard Reset button disable (elastic#159430)
  [Security Solution] Unskip Endpoint API tests after package fix (elastic#159484)
  [Synthetics] adjust alert timing (elastic#159511)
  [ResponseOps][rule registry] Remove usages of `refresh: true` (elastic#159252)
  Revert "Remove unused package (elastic#158597)"
  [Serverless] Adding config to disable authentication on task manager background worker utilization API (elastic#159505)
  Remove unused package (elastic#158597)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mistic mistic mistic approved these changes

Assignees

No one assigned

Labels

backport:skip This PR does not require backporting chore release_note:skip Skip the PR/issue when compiling release notes reverted v8.9.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@oatkiller @kibana-ci @mistic @kibanamachine