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

Upgrade cordova common #55

Merged
merged 2 commits into from
Sep 13, 2018
Merged

Upgrade cordova common #55

merged 2 commits into from
Sep 13, 2018

Conversation

erisu
Copy link
Member

@erisu erisu commented Sep 12, 2018

Platforms affected

osx

What does this PR do?

  • Bumps cordova-common to ^2.2.5

What testing has been done on this change?

  • npm test

@erisu
Copy link
Member Author

erisu commented Sep 12, 2018

Commits cherry-picked from @brodybits' PR #49

@erisu erisu requested a review from brodycj September 12, 2018 10:57
Copy link

@brodycj brodycj left a comment

Choose a reason for hiding this comment

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

Since master branch now targets next major release please use cordova-common@3.

@erisu
Copy link
Member Author

erisu commented Sep 12, 2018

@brodybits There is no cordova-common@3 in the NPM registry. I can't change it to something that doesn't exist.

Also, if this PR is not pushed, then NPM audit will always show security risk until we do a major release for cordova-common. I would like to to have at least the audit resolved. Even if we are not releasing the major this minute. We still build nightlies which would show these risks, though it should not affect to the users' app.

@brodycj
Copy link

brodycj commented Sep 12, 2018

Since cordova-common@3.0.0-nightly versions are already published, I would suggest the following alternatives:

@erisu
Copy link
Member Author

erisu commented Sep 12, 2018

From the dev mailing list discussion, I believe it was decided to not use nightly builds. If this is correct can the PR now be accepted so we can fix at least the audit? And worry about the version bump at release time?

@janpio
Copy link
Member

janpio commented Sep 12, 2018

Nothing here that could block a merge - if there is a better way to do things, this can be done in a new, additional PR anyway.

@brodycj
Copy link

brodycj commented Sep 12, 2018

Fine with me

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