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

CB-14145 resolve npm audit issues in 4.5.x patch fix #379

Merged
merged 12 commits into from
Jul 11, 2018

Conversation

brodycj
Copy link
Contributor

@brodycj brodycj commented Jul 6, 2018

Platforms affected

iOS

What does this PR do?

  • Set VERSION to 4.5.5-dev & update JS snapshot to version 4.5.5-dev via coho, using local coho with Improve patch release support cordova-coho#176 for patch release support
  • updated dependencies in package.json to resolve npm audit issues in this patch fix:
    • cordova-common@2.2.5
    • ios-sim@6.1.3
    • plist@2.1.0
  • completely reinstall node_modules (ignoring node_modules/.bin) using the following command on npm@6.1.0: npm install --only=production
  • Update cordova.js from cordova-js@4.2.4 with the following changes (using local coho with Improve patch release support cordova-coho#176 for patch release support):
    • CB-9366 log error.stack
  • update bundledDependencies to support deprecated Node.js 4 in this patch fix
  • remove devDependencies not needed in this patch fix:
    • coffee-script
    • uncrustify
  • update .travis.yml to test on latest version of Node.js 4 (deprecated), 6, 8, and 10 (with other minor cleanup fixes)
  • minor .gitignore updates in this patch fix
  • update appveyor.yml to cover Node.js 8 & 10 and do normal npm install now that workaround for uncrustify is no longer needed

What testing has been done on this change?

  • Adding local cordova-ios with these changes to new Cordova project on deprecated Node.js 4 results in no ugly engine warning errors, project runs on iOS (11.4)
  • Adding local cordova-ios with these changes to new Cordova project on Node.js 8 results in project that runs properly on iOS (11.4)
  • npm run unit-tests passes on full npm install on Node.js versions 4 (deprecated), 6, 8, and 10
  • npm audit with npm@6.1.0 (latest version) shows 0 vulnerabilities
  • using cordova platform add brodybits/cordova-ios#cb-14145-patch to add platform to new Cordova project and test on iOS simulator using cordova run ios succeeds on the following Node.js versions:
    • deprecated Node.js 4 (npm 2.15.11)
    • Node.js 8 (npm 5.6.0)

CI testing:

  • verify that npm test and other tests pass on Travis CI
  • verify that installation, unit-tests, etc. pass on AppVeyor CI

Checklist

  • Reported an issue in the JIRA database
  • Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected. (with some exceptions)
  • Added automated test coverage as appropriate for this change.

Christopher J. Brody added 4 commits July 3, 2018 19:28
update from cordova-js@4.2.4
- add Node.js 8 & 10
- show npm version
- latest releases of Node.js 4 & 6
- minor cleanup
@codecov-io
Copy link

codecov-io commented Jul 6, 2018

Codecov Report

Merging #379 into 4.5.x will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##            4.5.x     #379   +/-   ##
=======================================
  Coverage   63.45%   63.45%           
=======================================
  Files          14       14           
  Lines        1691     1691           
  Branches      284      284           
=======================================
  Hits         1073     1073           
  Misses        618      618

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 00a4b27...77bf23d. Read the comment docs.

@brodycj brodycj requested review from shazron, dpogue and raphinesse July 8, 2018 22:31
@brodycj brodycj changed the title CB-14145 resolve npm audit issues in patch fix [WIP] CB-14145 resolve npm audit issues in patch fix Jul 10, 2018
@brodycj
Copy link
Contributor Author

brodycj commented Jul 10, 2018

Marked as WIP since I would like to include xcode@1.0.1 once it is ready with changes fro apache/cordova-node-xcode#10 to resolve [dev] audit issues there.

P.S. My apologies for any possible confusion this may cause.

Christopher J. Brody added 5 commits July 11, 2018 10:09
- add Node.js versions 8 & 10
- do normal npm install (workaround for uncrustify no longer needed)
- coffee-script
- uncrustify
according to versions in node_modules
updates to resolve npm audit issues:
- cordova-common@2.2.5
- ios-sim@6.1.3
- plist@2.1.0
@brodycj brodycj changed the title [WIP] CB-14145 resolve npm audit issues in patch fix CB-14145 resolve npm audit issues in 4.5.x patch fix Jul 11, 2018
Christopher J. Brody added 3 commits July 11, 2018 11:43
items not needed (4.5.x):
- node_modules/.bin (all contents)
- node_modules/pegjs
- package-lock.json
as installed by the following command on npm@6.1.0:

    npm i --only=production

(needed to support deprecated Node.js 4 version)
(indirect production dependencies needed by deprecated Node.js 4.x)
@brodycj brodycj merged commit 60be403 into apache:4.5.x Jul 11, 2018
@brodycj brodycj deleted the cb-14145-patch branch July 11, 2018 16:02
@brodycj
Copy link
Contributor Author

brodycj commented Jul 11, 2018

Merged with a few additional changes:

  • pegjs from cordova-node-xcode which is not needed in production is not included in node_modules
  • changes not wanted in the master branch are marked "4.5.x only"

Thanks @shazron for the review.

@brodycj brodycj removed the request for review from dpogue July 11, 2018 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants