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

Do not explicitly require modules from project directory #713

Merged
merged 2 commits into from
Apr 13, 2019

Conversation

raphinesse
Copy link
Contributor

@raphinesse raphinesse commented Apr 12, 2019

Motivation and Context

Before this change, the following would fail if cordova-android or one of its dependencies are not resolvable from dir:

require('cordova-android').createPlatform(dir, configParser, null, events)

This happens for example if you want to setup a platform-centered App from scratch. In my case I wanted to create a project in a temp folder during plugman test setups in cordova-lib.

The problem is, that during project creation cordova-android requires some of the modules that have been checked-out to the project directory instead of the ones relative to the running modules (looking at the actual changes will probably clarify what I'm trying to say here).

Description

Instead of switching to the checked-out modules in the created platform-project, we simply use the identical modules relative to the active ones.

Testing

Ran the automated test suite. Ran tests in cordova-lib that use cordova-android using this updated version.

@raphinesse raphinesse added the bug label Apr 12, 2019
@raphinesse raphinesse added this to the 8.0.1 milestone Apr 12, 2019
@raphinesse raphinesse requested review from dpogue and erisu April 12, 2019 20:36
@codecov-io
Copy link

codecov-io commented Apr 12, 2019

Codecov Report

Merging #713 into master will increase coverage by 2.59%.
The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #713      +/-   ##
==========================================
+ Coverage   63.98%   66.57%   +2.59%     
==========================================
  Files          18       18              
  Lines        1824     1822       -2     
==========================================
+ Hits         1167     1213      +46     
+ Misses        657      609      -48
Impacted Files Coverage Δ
bin/lib/create.js 92.72% <100%> (ø) ⬆️
bin/templates/cordova/lib/builders/builders.js 100% <100%> (ø) ⬆️
bin/templates/cordova/Api.js 53.48% <50%> (+2.35%) ⬆️
...n/templates/cordova/lib/builders/ProjectBuilder.js 69.64% <0%> (+26.78%) ⬆️

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 b177f84...a2b24de. Read the comment docs.

@raphinesse raphinesse merged commit 4cf3dcf into apache:master Apr 13, 2019
@raphinesse raphinesse deleted the no-require-from-project branch April 13, 2019 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants