Skip to content
This repository has been archived by the owner on Mar 21, 2022. It is now read-only.

Dependency bump cordova-common@^3.0.0 #69

Merged
merged 1 commit into from
Nov 8, 2018
Merged

Conversation

erisu
Copy link
Member

@erisu erisu commented Nov 8, 2018

Platforms affected

osx

What does this PR do?

Updates the cordova-common dependency to ^3.0.0.

What testing has been done on this change?

@erisu erisu requested a review from dpogue November 8, 2018 01:16
@dpogue
Copy link
Member

dpogue commented Nov 8, 2018

Are we intentionally committing package-lock.json in this repo? If so, it might be worth deleting and regenerating it because I'm not sure the diff here is accurate...

},
"acorn-jsx": {
"version": "3.0.1",
"resolved": "https://registry.npmjs.org/acorn-jsx/-/acorn-jsx-3.0.1.tgz",
"resolved": "http://registry.npmjs.org/acorn-jsx/-/acorn-jsx-3.0.1.tgz",
Copy link

Choose a reason for hiding this comment

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

As a minor comment, I find it strange that it goes from https to http here. Looks like something that should be fixed in the npm tooling.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that's a known npm bug. It doesn't really matter because npm always uses HTTPS for fetching from the registry, but they're still trying to track down why it happens.

@erisu
Copy link
Member Author

erisu commented Nov 8, 2018

@dpogue

I recall the package-lock was regenerated, but I deleted the node_modules and package-lock.json one more time and reinstalled to create a new package-lock.json. Also has been rebased.

As for intentionally committing package-lock.json, it file was already committed so I was not deleting what was already there.

I know there is an open-ended discussion about if these files should be committed or not. As I said before, I am still in favor for committing them. But in this case, it was already committed and I only updated the existing file.

@brodycj
Copy link

brodycj commented Nov 8, 2018

Are we intentionally committing package-lock.json in this repo?

Yes, I think we reached agreement in the following discussions in the past:

There was some dissent, mostly from a non-member. I did raise some questions and minor concerns. I did commit package-lock.json in a few places despite my own concerns since everyone else seemed to want it. In case of any further questions or concerns I would favor discussing in the existing thread or a new issue.

@dpogue dpogue merged commit 8a70ced into apache:master Nov 8, 2018
@erisu erisu deleted the bump-common-3x branch April 4, 2019 06:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants