Github user shazron commented on the issue:
https://github.com/apache/cordova-ios/pull/234
Interesting node quirk. I think I figured out the problem with this line:
https://github.com/apache/cordova-ios/pull/234/files#diff-4dd01b7c4b3f825b9b9e6f223a83ec71R97
in node 0.10 and
Github user shazron commented on the issue:
https://github.com/apache/cordova-ios/pull/234
LGTM! Congrats on landing your first feature into Cordova @juliascript !
(note the CI failures are for node 0.1x.x which I think we can safely say
we won't support anymore)
---
If your
Github user shazron commented on the issue:
https://github.com/apache/cordova-ios/pull/234
1 unit-test failure:
```
Failures:
1) unit tests for pod module tear down encountered a declaration exception
Message:
Error: ENOENT: no such file or directory,
Github user shazron commented on the issue:
https://github.com/apache/cordova-ios/pull/234
Recapping our chat:
5 unit test failures:
```
Failures:
1) installPodSync throws cordova error when no pod name provided
Message:
Error: ENOENT: no
Github user shazron commented on the issue:
https://github.com/apache/cordova-ios/pull/234
Sadly Jasmine 1.x doesn't have an `afterAll` global function unlike Jasmine
2.x. You would have to nest the `describe` tests in a `describe`, sandwiched
between two `it`s, the first is the
Github user shazron commented on the issue:
https://github.com/apache/cordova-ios/pull/234
The `podMod.spec.js` tests should have a âcleanupâ after all tests are
run (not sure how in Jasmine 1.0) which should remove the test products
Github user shazron commented on the issue:
https://github.com/apache/cordova-ios/pull/234
Upon further review - the Podfile should not be added. The unit tests
actually show the problem -- the Podfile was not created. The `spyOn` for
`writeFileSync` in the tests should have a
Github user shazron commented on the issue:
https://github.com/apache/cordova-ios/pull/234
Tests fail. I cloned cordova-ios, applied the PR patch, npm install then
npm test.
Perhaps you forgot to check in
`tests/spec/unit/fixtures/testProj/platforms/ios/Podfile`?
Log:
Github user shazron commented on the issue:
https://github.com/apache/cordova-ios/pull/234
Can't run the tests, I get:
```
Exception loading:
/Users/shazron/Desktop/cordova-ios/tests/spec/unit/podMod.spec.js
{ Error: Cannot find module
Github user shazron commented on the issue:
https://github.com/apache/cordova-ios/pull/234
Looking at this now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and
Github user shazron commented on the issue:
https://github.com/apache/cordova-ios/pull/234
I'm afraid this PR needs to be re-based with master. I had to pull in other
pull requests that probably touched part of the code you were working on.
---
If your project is set up for it, you
Github user blakgeek commented on the issue:
https://github.com/apache/cordova-ios/pull/234
@stevengill I totally agree with this approach. I specifically didn't use
the framework tag for my plugin because I didn't want it to clash with a native
implementation in the future, like
Github user stevengill commented on the issue:
https://github.com/apache/cordova-ios/pull/234
@blakgeek that is pretty cool!
Looks like @juliascript PR handles most of it. (doesn't handle
configuration yet)
For android, you would add your plugin deps to your gradle
Github user blakgeek commented on the issue:
https://github.com/apache/cordova-ios/pull/234
@juliascript @shazron Have you considered adding support for specifying
pods at the project level as well. This makes it easy to resolve conflicts in
pod specifications across plugins and is
Github user shazron commented on the issue:
https://github.com/apache/cordova-ios/pull/234
There still need to be tests for the new code that is introduced in this
repo (platform API etc), or perhaps I'm misunderstanding your last comment Steve
---
If your project is set up for it,
Github user stevengill commented on the issue:
https://github.com/apache/cordova-ios/pull/234
Tests for this are at https://github.com/apache/cordova-lib/pull/467
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user juliascript commented on the issue:
https://github.com/apache/cordova-ios/pull/234
Build failing bc incomplete tests.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this
Github user codecov-io commented on the issue:
https://github.com/apache/cordova-ios/pull/234
## [Current coverage][cc-pull] is **48.34%**
> Merging [#234][cc-pull] into [master][cc-base-branch] will decrease
coverage by **3.47%**
```diff
@@ master
Github user juliascript commented on the issue:
https://github.com/apache/cordova-ios/pull/234
Writing tests currently. Thank you, @shazron!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user shazron commented on the issue:
https://github.com/apache/cordova-ios/pull/234
No tests for this feature were available. Tests will need to be added
before this can land.
The existing tests pass now.
---
If your project is set up for it, you can reply to this email
Github user shazron commented on the issue:
https://github.com/apache/cordova-ios/pull/234
Failing tests. Please run `npm install && npm test` in the repo first
before committing. Looks like it fails the jshint tests.
---
If your project is set up for it, you can reply to this email
21 matches
Mail list logo
