Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

CB-12189: Add support for WinMD and DLL combination #219

Closed

Conversation

matrosov-nikita
Copy link
Contributor

Platforms affected

self

What does this PR do?

This PR adds support for .winmd and .dll combination.

What testing has been done on this change?

Auto-test

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.
  • Added automated test coverage as appropriate for this change.

@codecov-io
Copy link

codecov-io commented Dec 21, 2016

Current coverage is 77.00% (diff: 100%)

Merging #219 into master will increase coverage by 0.77%

@@             master       #219   diff @@
==========================================
  Files            16         16          
  Lines          2204       2222    +18   
  Methods         412        415     +3   
  Messages          0          0          
  Branches        433        438     +5   
==========================================
+ Hits           1680       1711    +31   
+ Misses          524        511    -13   
  Partials          0          0          

Powered by Codecov. Last update 1cf27a9...110bf79

@vladimir-kotikov
Copy link
Member

Documentation update: apache/cordova-docs#671
Change to support implementation attribute in cordova-common: apache/cordova-lib#513

Note - this PR has own override for PluginInfo.getFrameworks method to not depend on not-yet-released apache/cordova-lib#513 - this logic can be safely removed once updated cordova-common released

@matrosov-nikita matrosov-nikita changed the title Add support for WinMD and DLL combination CB-12189: Add support for WinMD and DLL combination Dec 21, 2016
@vladimir-kotikov
Copy link
Member

@daserge could you take a look please?

Copy link
Contributor

@daserge daserge left a comment

Choose a reason for hiding this comment

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

Minor comments, otherwise LGTM 👍

@@ -115,6 +115,33 @@ function PluginInfo(dirname) {
var configFiles = parentGetConfigFiles(platform);
return processChanges(configFiles);
};

this.getFrameworks = function(platform) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@@ -304,6 +309,47 @@ describe('windows project handler', function () {
xpath = 'Reference[@Include="dummy6"]/HintPath';
validateInstalledProjects('framework', frameworks[5], xpath, ['windows', 'windows10', 'phone']);
});

it('with .winmd and .dll files', function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this spec check that dll files have been actually copied? (according to this)

@matrosov-nikita
Copy link
Contributor Author

@vladimir-kotikov, @daserge, updated.

Copy link
Contributor

@daserge daserge left a comment

Choose a reason for hiding this comment

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

👍

@vladimir-kotikov
Copy link
Member

Rebased on top of master and force-pushed, will merge once tests pass

@asfgit asfgit closed this in ae433f6 Dec 23, 2016
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.

4 participants