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

Upgrade all minimist (sub)dependencies to version ^1.2.5 #60284

Merged
merged 6 commits into from
Sep 17, 2020
Merged

Upgrade all minimist (sub)dependencies to version ^1.2.5 #60284

merged 6 commits into from
Sep 17, 2020

Conversation

watson
Copy link
Contributor

@watson watson commented Mar 16, 2020 •
edited
Loading

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.

@watson watson added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.7.0 labels Mar 16, 2020
@watson watson self-assigned this Mar 16, 2020
@watson watson requested a review from a team as a code owner March 18, 2020 10:24
spalger
spalger reviewed Mar 18, 2020
View reviewed changes
package.json Outdated Show resolved Hide resolved
@watson watson added v7.8.0 and removed v7.7.0 labels Mar 25, 2020
jportner
jportner reviewed Mar 27, 2020
View reviewed changes
Copy link
Contributor

@jportner jportner left a comment

Choose a reason for hiding this comment

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

Thoughts?

yarn.lock Outdated Show resolved Hide resolved
yarn.lock Outdated Show resolved Hide resolved
yarn.lock Outdated Show resolved Hide resolved
@watson watson force-pushed the minimist branch 2 times, most recently from 42a1ce6 to a8e5951 Compare March 28, 2020 10:27
@watson watson mentioned this pull request Apr 27, 2020
Merged
@watson watson added v7.9.0 and removed v7.8.0 labels May 28, 2020
@watson watson added v7.10.0 and removed v7.9.0 labels Aug 11, 2020
@watson watson changed the title Upgrade all minimist (sub)dependencies to version ^1.2.5 Upgrade all minimist (sub)dependencies to version ^0.2.1/^1.2.5 Sep 16, 2020
@watson
Copy link
Contributor Author

watson commented Sep 16, 2020

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.

@watson watson requested review from jportner and spalger September 16, 2020 12:56
@jportner
Copy link
Contributor

@elasticmachine merge upstream

elasticmachine and others added 2 commits September 16, 2020 19:56
@elasticmachine
Merge branch 'master' into minimist
b0013c9
@jportner
Consolidate last minimist dev dependency
1d86c9c 
This was a transitive dependency:
@mapbox/geojson-rewind@^0.4.1 > sharkdown@^0.1.0 > [email protected]
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.
jportner
jportner approved these changes Sep 17, 2020
View reviewed changes
Copy link
Contributor

@jportner jportner left a 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!

@watson
Copy link
Contributor Author

watson commented Sep 17, 2020

@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.

@watson
Copy link
Contributor Author

watson commented Sep 17, 2020

@elasticmachine merge upstream

@watson
Copy link
Contributor Author

watson commented Sep 17, 2020

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 ^1.2.5.

@watson watson changed the title Upgrade all minimist (sub)dependencies to version ^0.2.1/^1.2.5 Upgrade all minimist (sub)dependencies to version ^1.2.5 Sep 17, 2020
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
fileUpload 27 -2 29

async chunks size

id value diff baseline
enterpriseSearch 434.6KB +23.0B 434.6KB
fileUpload 720.7KB -1.5KB 722.2KB
total -1.5KB

page load bundle size

id value diff baseline
upgradeAssistant 64.9KB +23.0B 64.9KB

History

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

@watson watson merged commit f81901e into elastic:master Sep 17, 2020
@watson watson deleted the minimist branch September 17, 2020 09:14
watson added a commit to watson/kibana that referenced this pull request Sep 17, 2020
@watson @jportner
Upgrade all minimist (sub)dependencies to version ^1.2.5 (elastic#60284)
5c7599b 
Co-authored-by: Joe Portner <[email protected]>
@watson watson mentioned this pull request Sep 17, 2020
Merged
watson added a commit that referenced this pull request Sep 17, 2020
@watson @jportner
Upgrade all minimist (sub)dependencies to version ^1.2.5 (#60284) (#7…
475ba3e 
…7729)

Co-authored-by: Joe Portner <[email protected]>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Sep 18, 2020
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into ingest-node/g…
6a85bac 
…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
@watson watson mentioned this pull request Jun 15, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jportner jportner jportner approved these changes

@spalger spalger Awaiting requested review from spalger

Assignees

@watson watson

Labels
release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@watson @jportner @kibanamachine @spalger @elasticmachine