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

Added Back Missing unorm Dependency #406

Merged
merged 1 commit into from
Sep 12, 2018

Conversation

erisu
Copy link
Member

@erisu erisu commented Sep 12, 2018

Platforms affected

ios

What does this PR do?

The goal of this PR is to re-add the missing dependency so cordova-ios does not fail with cordova-common master.

cordova-common use to have a dependency called unorm. This dependency was not being used in cordova-common so I suspect it was removed.

apache/cordova-common@d473b19#diff-b9cfc7f2cdf78a7f4b91a753d10865a2

This dependency, on the other hand, was being used in cordova-ios.

var unorm = require('unorm');

var unorm = require('unorm');

Previously, unorm use to to be a committed node_module for cordova-ios repo. Once the committed node_modules were removed, both cordova-ios and cordova-common on the master branch were missing this dependency.

If you clone cordova-ios master, run npm install, and then run the tests for ios, it does not fail is because cordova-ios package.json is configured to use [email protected] which has unorm. To reproduce, make sure to also update cordova-common to point to master.

What testing has been done on this change?

  • npm run

Test failures may appear when running against cordova-common@master. This is not related to adding unorm. I had also tested using the older version of [email protected], which is identical to what has been committed into the repo previously.

Those issues will need to be invested and fixed in a separate PR.

@erisu erisu requested a review from dpogue September 12, 2018 10:40
@codecov-io
Copy link

codecov-io commented Sep 12, 2018

Codecov Report

Merging #406 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #406   +/-   ##
=======================================
  Coverage   74.29%   74.29%           
=======================================
  Files          12       12           
  Lines        1564     1564           
=======================================
  Hits         1162     1162           
  Misses        402      402

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 ff1bcd0...aea6b4a. Read the comment docs.

@dpogue dpogue merged commit 1117061 into apache:master Sep 12, 2018
@erisu erisu deleted the add-missing-unorm-dependency branch April 4, 2019 06:09
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.

4 participants