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.

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

Bump merge-deep from 3.0.2 to 3.0.3 #659

Merged
merged 1 commit into from
Jun 15, 2021

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Jun 8, 2021

Bumps merge-deep from 3.0.2 to 3.0.3.

Commits
  • 628ff47 3.0.3
  • cfbe20c run verb to generate README documentation
  • e370968 Merge pull request #17 from jonschlinkert/key-properties
  • 393e2cb adding a test to ensure using merge-deep for inheritance still works
  • c39b161 add test to ensure constructor is not cloned
  • 11e5dd5 add isValidKey function to ensure only valid keys are merged
  • See full diff in compare view

Dependabot compatibility score

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
  • @dependabot use these labels will set the current labels as the default for future PRs for this repo and language
  • @dependabot use these reviewers will set the current reviewers as the default for future PRs for this repo and language
  • @dependabot use these assignees will set the current assignees as the default for future PRs for this repo and language
  • @dependabot use this milestone will set the current milestone as the default for future PRs for this repo and language

You can disable automated security fix PRs for this repo from the Security Alerts page.

@dependabot dependabot bot added the Dependencies Pull requests that update a dependency file label Jun 8, 2021
@htdat htdat self-assigned this Jun 9, 2021
@htdat
Copy link
Member

htdat commented Jun 10, 2021

merge-deep is a nested package then only package-lock.json.

I think we should not merge this PR as a direct update to package-lock.json does not guarantee the same result when running npm ci or npm install next times.

Update: should not be an issue with npm ci.

This answer makes sense to me https://stackoverflow.com/a/48263958

I think we may spend time auditing and update all npm packages in the package.json file. I am looking forward to seeing other ideas/thoughts for this.

Copy link
Member

@nielslange nielslange left a comment

Choose a reason for hiding this comment

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

When running the following commands

rm -rf node_modules
rm -rf package-lock.json
npm i --legacy-peer-deps

and then looking up package-lock.json, I see that the version of merge-deep had been bumped from 3.0.2 to 3.0.3. Therefore, I don't see a problem merging this PR.

@htdat
Copy link
Member

htdat commented Jun 11, 2021

npm i --legacy-peer-deps

TIL legacy-peer-deps. I will check if it this affects anything to CI.

@htdat
Copy link
Member

htdat commented Jun 11, 2021

@dependabot recreate

@htdat
Copy link
Member

htdat commented Jun 11, 2021

@dependabot rebase

Bumps [merge-deep](https://github.com/jonschlinkert/merge-deep) from 3.0.2 to 3.0.3.
- [Release notes](https://github.com/jonschlinkert/merge-deep/releases)
- [Commits](jonschlinkert/merge-deep@3.0.2...3.0.3)

---
updated-dependencies:
- dependency-name: merge-deep
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
@dependabot dependabot bot force-pushed the dependabot/npm_and_yarn/merge-deep-3.0.3 branch from 7387d09 to 19ce846 Compare June 11, 2021 03:40
@htdat
Copy link
Member

htdat commented Jun 11, 2021

@natebot and @mikeyarce would love to see your thoughts regarding this, especially Niels' comment #659 (review)

@mikeyarce
Copy link
Member

Looked into this and found this:

Screen Shot on 2021-06-11 at 13:39:56

It looks like the main package that we need to update is @wordpress/scripts - I tried updating to the latest 16.1.0 and it seemed to work, but I'm not sure how to test it entirely.
Here's the changelog for @wordpress/scripts: https://github.com/WordPress/gutenberg/blob/HEAD/packages/scripts/CHANGELOG.md

@htdat what do you think might be a sensible plan here? Should we try to update to 16.1.0 and test?

@htdat
Copy link
Member

htdat commented Jun 14, 2021

@mikeyarce - TIL npm ls command.

It looks like the main package that we need to update is @wordpress/scripts - I tried updating to the latest 16.1.0 and it seemed to work, but I'm not sure how to test it entirely.

All that's right. It's working for my local machine and GitHub Actions https://github.com/htdat/Edit-Flow/runs/2818219871

Here's the changelog for @wordpress/scripts: https://github.com/WordPress/gutenberg/blob/HEAD/packages/scripts/CHANGELOG.md
@htdat what do you think might be a sensible plan here? Should we try to update to 16.1.0 and test?

I've found out that my PR here #645 was only to update package versions that satisfy satisfy what is mentioned in package.json. That is, ^4.0.2 means npm update will only look for versions between 4.0.2 and any newer version with this format 4.x.x. @nielslange somewhat mentioned this here #645 (review)

IMHO, a sensible plan would be:

  • Merge this PR.
  • Create a follow-up issue to update all versions in package.json to the latest versions or most newest possible ones. I expect this is kinda a big task. Because we will need to (1) fix incompatibility issues across dependencies, and (2) ensure our tests and GH Actions run properly.

Copy link
Member

@mikeyarce mikeyarce left a comment

Choose a reason for hiding this comment

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

👍🏽

@htdat htdat merged commit c7e3fa7 into master Jun 15, 2021
@htdat htdat deleted the dependabot/npm_and_yarn/merge-deep-3.0.3 branch June 15, 2021 04:26
@htdat
Copy link
Member

htdat commented Jun 15, 2021

Created this follow-up issue #671

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants