Skip to content
This repository has been archived by the owner on Oct 1, 2018. It is now read-only.

Refactored to use centralized downloader. #40

Merged

Conversation

davidovich
Copy link
Contributor

Implemented using NSURLSession which can be configured with headers. (In
a future PR).

Removed now unused network related files.

Fixes #36

@davidovich davidovich force-pushed the use_NSURLSession_to_allow_auth branch from e553c77 to 3b01c28 Compare November 19, 2015 01:37
@nikDemyankov
Copy link
Member

Thanks for that, will look into it.

@davidovich
Copy link
Contributor Author

I noticed only later that the plugin.xml file would have to be updated with regards to the file deletions I made. I will submit an addendum soon.

Is there an intent to add a testing framework and tests? Because we are pretty much in the dark for changes of this nature. I am not sure of the current best practice on cordova plugin testing.

@nikDemyankov
Copy link
Member

Regarding the plugin.xml file - yes, I have noticed that, but it's not a problem.

Regarding tests: I have that in plans for the future, but how exactly this will and should work - don't know yet. Didn't have much time for investigation. There should be some unit tests for the native side and for the hooks, plus some integration tests. Unit tests for the native should be written by using the platform instruments for that. Tests for hooks and integration - on JS. And we need a way to run all of them from console. With JS tests that's easy - npm test. With a native side it is a bit harder, but should be solvable.

@nikDemyankov nikDemyankov added this to the v1.1.2 milestone Nov 23, 2015
Implemented using NSURLSession which can be configured with headers. (In
a future PR).

Removed now unused network related files.
@davidovich davidovich force-pushed the use_NSURLSession_to_allow_auth branch from 3b01c28 to 4fe4629 Compare November 25, 2015 03:29
@nikDemyankov nikDemyankov merged commit 4fe4629 into nordnet:master Nov 30, 2015
@nikDemyankov
Copy link
Member

Merged with some cleanup. Thanks!

@davidovich
Copy link
Contributor Author

Thank you @nikDemyankov,

I have the PR for allowing headers ready. Will post shortly.

Do not hesitate to forward any problems to me.

@nikDemyankov
Copy link
Member

Cool, thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants