-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Upgrade all minimist (sub)dependencies to version ^1.2.5 #60284
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thoughts?
42a1ce6
to
a8e5951
Compare
I resurrected this old PR today, rebased with master, fixed a lot of conflicts and cleaned it up quite a bit. It should be fairly simple to review again. |
@elasticmachine merge upstream |
This was a transitive dependency: @mapbox/geojson-rewind@^0.4.1 > sharkdown@^0.1.0 > minimist@0.0.5 The sharkdown package had a Yarn dependency resolution to bump minimist to ^0.2.1. By upgrading @mapbox/geojson-rewind, we are able to get rid of all of this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
I noticed that we could get rid of the final minimist dev dependency and several other transitive dependencies by upgrading the @mapbox/geojson-rewind package from 0.4.1 to 0.5.0. I took a look at the repo, I couldn't find a changelog, but the changes between the two versions appear to be very minimal.
Normally I wouldn't do this without asking first, but since it's a simple change, figured I would save you some trouble and push the change so it can finish CI before you log on tomorrow. Details in 1d86c9c. If you'd rather not make that change, by all means please feel free to revert it and merge without it!
@jportner nice find! I thought I went over all of these yesterday to see if they had been upgraded in the meantime. But somehow I must have missed this one. Here's a complete diff of the changes between 0.4.1 and 0.5.0: mapbox/geojson-rewind@v0.4.1...v0.5.0 As far as I can see from the commits, the breaking change introduced in v0.5.0 is related to the CLI which we do not use. So I think the test-failure is unrelated - I'll just try to re-run them and see what happens. |
@elasticmachine merge upstream |
Since sharkdown is no more, we don't have to worry about upgrading its minimist version. So I've changed all the forced upgrades to a catch-all that always upgrades minimist to |
💚 Build SucceededBuild metrics@kbn/optimizer bundle module count
async chunks size
page load bundle size
History
To update your PR or re-run it, just comment with: |
Co-authored-by: Joe Portner <5295965+jportner@users.noreply.github.com>
…rok/new-patterns-component-use-array * 'master' of github.com:elastic/kibana: (140 commits) Add telemetry as an automatic privilege grant (elastic#77390) [Security Solutions][Cases] Cases Redesign (elastic#73247) Use Search API in TSVB (elastic#76274) [Mappings editor] Add support for constant_keyword field type (elastic#76564) [ML] Adds ML modules for Metrics UI Integration (elastic#76460) [Drilldowns] {{event.points}} in URL drilldown for VALUE_CLICK_TRIGGER (elastic#76771) Migrate status & stats APIs to KP + remove legacy status lib (elastic#76054) use App updater API instead of deprecated chrome.navLinks.update (elastic#77708) [CSM Dashboard] Remove points from line chart (elastic#77617) [APM] Trace timeline: Replace multi-fold function icons with new EuiIcon glyphs (elastic#77470) [Observability] Overview: Alerts section style improvements (elastic#77670) Bump the Node.js version used by Docker in CI (elastic#77714) Upgrade all minimist (sub)dependencies to version ^1.2.5 (elastic#60284) Remove unneeded forced package resolutions (elastic#77467) [ML] Add metrics app to check made for internal custom URLs (elastic#77627) Functional tests - add supertest for test_user (elastic#77584) [ML] Adding option to create AD jobs without starting the datafeed (elastic#77484) Bump node-fetch to 2.6.1 (elastic#77445) Bump sharkdown from v0.1.0 to v0.1.1 (elastic#77607) [APM]fixing y axis on transaction error rate to 100% (elastic#77609) ... # Conflicts: # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/manage_processor_form.container.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/manage_processor_form.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processor_form/field_components/drag_and_drop_text_list.scss # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processor_form/field_components/drag_and_drop_text_list.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processor_form/field_components/text_editor.scss # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processor_form/processors/grok.test.tsx
The earliest version we used was v0.0.5, so I checked for commits introducing breaking changes between v0.0.5 and v1.0.0 and didn't find any significant braking change that I think affects us. All of the breaking changes that were introduced were related to how the input string is parsed - not related to the
minimist
API, which has remained unchanged.In many cases,
minimist
is not even used in our use-case, but is simply present as a dependency in case people use the CLI version of a module instead of using it programmatically.However, a patch to minimist was released for versions prior to 1.0.0, so those modules that depend on a 0.x version has been forced to use ^0.2.1 (except
mkdirp
which is know to work fine with ^1.2.5). All modules using version 1.x of minimist are now running ^1.2.5.