Skip to content

Update lodash imports #1724

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
Closed
vojtechszocs wants to merge 4 commits into from
Closed

Update lodash imports #1724

vojtechszocs wants to merge 4 commits into from

Conversation

@vojtechszocs
Copy link
Contributor

@vojtechszocs vojtechszocs commented Jun 14, 2019 •
edited
Loading

Follow-up to #1708, this PR removes the lodash vs. lodash-es dependency duality.

During webpack build, lodash imports get transformed into corresponding lodash-es imports.

dependencies

  • added explicit lodash@4.x alongside existing lodash-es@4.x
  • both ^^ now resolve to the same version
$ cat node_modules/lodash/package.json | jq .version
"4.17.11"
$ cat node_modules/lodash-es/package.json | jq .version
"4.17.11"

devDependencies

  • removed @types/lodash-es
  • added @types/lodash-es

The last commit in this PR replaces the remaining lodash-es imports with lodash.

@openshift-ci-robot openshift-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 14, 2019
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jun 14, 2019

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: vojtechszocs
To complete the pull request process, please assign bparees
You can assign the PR to them by writing /assign @bparees in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@vojtechszocs
Copy link
Contributor Author

vojtechszocs commented Jun 14, 2019

/cc @spadgett

This was referenced Jun 14, 2019
Merged
Merged
benjaminapetersen
benjaminapetersen reviewed Jun 14, 2019
View reviewed changes
frontend/packages/console-app/src/__tests__/extension-checks/nav-items.spec.ts Outdated
Copy link
Contributor

@benjaminapetersen benjaminapetersen Jun 14, 2019

Choose a reason for hiding this comment

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

This PR looks like more than updates to lodash imports?

Copy link
Contributor Author

@vojtechszocs vojtechszocs Jun 14, 2019

Choose a reason for hiding this comment

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

Hi Ben, this PR is a follow-up to un-merged #1708, above change comes from that PR.

@benjaminapetersen
Copy link
Contributor

benjaminapetersen commented Jun 14, 2019

Several hundred files, it may be helpful to split into 2 commits, the first for the configuration changes you made, the second for all the files that are updated. I'm sure the configuration changes are what need the review attention.

@vojtechszocs
Copy link
Contributor Author

vojtechszocs commented Jun 14, 2019

Several hundred files, it may be helpful to split into 2 commits, the first for the configuration changes you made, the second for all the files that are updated. I'm sure the configuration changes are what need the review attention.

Agreed, will do.

@spadgett spadgett added this to the v4.2 milestone Jun 14, 2019
@vojtechszocs vojtechszocs force-pushed the update-lodash-imports branch from a279b53 to f8b9912 Compare June 18, 2019 16:35
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 18, 2019
@vojtechszocs
Copy link
Contributor Author

vojtechszocs commented Jun 18, 2019 •
edited
Loading

The first 2 commits come from different un-merged PRs, please review the last 2 commits only.

@vojtechszocs vojtechszocs force-pushed the update-lodash-imports branch from f8b9912 to 2abc372 Compare June 18, 2019 16:58
@vojtechszocs
Copy link
Contributor Author

vojtechszocs commented Jun 18, 2019

/retest

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 18, 2019
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jun 18, 2019

@vojtechszocs: PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@vojtechszocs
Copy link
Contributor Author

vojtechszocs commented Jun 18, 2019

/retest

@vojtechszocs
Copy link
Contributor Author

vojtechszocs commented Jun 20, 2019

Putting this on hold for now, I'll extract the dependency update commit into a separate PR.

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 20, 2019
@spadgett spadgett removed this from the v4.2 milestone Jul 29, 2019
@priley86 priley86 mentioned this pull request Jul 31, 2019
Closed
@aljesusg aljesusg mentioned this pull request Aug 5, 2019
Closed
@openshift-bot
Copy link
Contributor

openshift-bot commented Sep 20, 2020

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci-robot openshift-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 20, 2020
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 6, 2020

@vojtechszocs: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/backend 2abc372 link /test backend
ci/prow/e2e-aws-console-olm 2abc372 link /test e2e-aws-console-olm
ci/prow/e2e-aws 2abc372 link /test e2e-aws
ci/prow/e2e-aws-console 2abc372 link /test e2e-aws-console
ci/prow/images 2abc372 link /test images
ci/prow/frontend 2abc372 link /test frontend
ci/prow/e2e-gcp-console 2abc372 link /test e2e-gcp-console
ci/prow/e2e-gcp 2abc372 link /test e2e-gcp
ci/prow/analyze 2abc372 link /test analyze
ci/prow/kubevirt-plugin 2abc372 link /test kubevirt-plugin

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 6, 2020

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci-robot openshift-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Nov 6, 2020
@openshift-bot
Copy link
Contributor

openshift-bot commented Dec 6, 2020

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Dec 6, 2020

@openshift-bot: Closed this PR.

Details

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alecmerdler alecmerdler Awaiting requested review from alecmerdler

@gallettilance gallettilance Awaiting requested review from gallettilance

@spadgett spadgett Awaiting requested review from spadgett

1 more reviewer

@benjaminapetersen benjaminapetersen benjaminapetersen left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@vojtechszocs @openshift-ci-robot @benjaminapetersen @openshift-bot @spadgett