Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-file-transfer/pull/171
oo, nice catch!
However, I fail to see how the test added is exercising the `abort()` code
path? I'm not actually sure what the test is doing other than ensur
Github user filmaj commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-camera/pull/254#discussion_r102069422
--- Diff: appium-tests/ios/ios.spec.js ---
@@ -82,11 +86,43 @@ describe('Camera tests iOS.', function () {
.elem
Github user filmaj commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-camera/pull/254#discussion_r102245774
--- Diff: appium-tests/ios/ios.spec.js ---
@@ -82,11 +86,43 @@ describe('Camera tests iOS.', function () {
.elem
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-contacts/pull/146
It _does_ look like [iOS returns label
information](https://github.com/apache/cordova-plugin-contacts/blob/master/src/ios/CDVContact.h#L92)
in contacts, so there is precedent to
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-contacts/pull/142
It looks like a JIRA issue is filed for this already:
https://issues.apache.org/jira/browse/CB-10496
---
If your project is set up for it, you can reply to this email and have your
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-contacts/pull/142
Confirmed that this fixes the photo issue - tested on an Android 5.1
emulator.
@infil00p any qualms with merging this in?
---
If your project is set up for it, you can
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-camera/pull/255
Would be nice to get rid of the multiple-calls-to-elements-via-xpath :D
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-contacts/pull/134
Let there be 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 feature
enabled
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-contacts/pull/130
Please see the discussion in
[CB-11532](https://issues.apache.org/jira/browse/CB-11532) for details on the
issue.
---
If your project is set up for it, you can reply to this email
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-contacts/pull/109
Ping @shazron - what do you think of this? Looks like we're forcing
decoding base64 image data in a contact to help with an alleged iCloud sync
issue.
---
If your project is s
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-contacts/pull/107
Ping @shazron once more for a quick review, please and thank you!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-contacts/pull/103
This pull request is either malformed or a mistake happened. There is no
code here to test!
Closing this.
---
If your project is set up for it, you can reply to this email
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-contacts/pull/102
This pull request has merge conflicts at this point, and I believe a
more-complete pull request satisfying this functionality (without merge
conflicts) is already live at #146
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-contacts/pull/88
FWIW, the latest UI automation framework on iOS 9.3+, XCUITest, _should_
allow interacting with built-in/preinstalled applications. However, in practice
we won't be able to eleg
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-contacts/pull/71
There are merge conflicts in this pull request, and I believe a more
complete and up-to-date implementation is in #146 (along with try/catch
protection when interacting with the
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-contacts/pull/70
As discussed, the issue in question should be discussed and a problem
identified first before a solution is proposed.
If this is still a problem, please head over to
https
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-contacts/pull/81
Ping @gtanner for review please and thank you ;)
---
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
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-contacts/pull/51
Until @Hurtyto signs the ICLA, we cannot accept this patch :(
---
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 filmaj commented on the issue:
https://github.com/apache/cordova-plugin-contacts/pull/134
@soonahn it looks like the tests are hanging in this pull request, across
all platforms, surprisingly! I think it may be because your branch is ~25
commits behind master
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-contacts/pull/10
Until we get an ICLA from @huanghf, we cannot do much. There are also merge
conflicts in this 3.5 year old pull request.
I will close this pull request. @huanghf if you have
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-contacts/pull/146
Thanks for the review @infil00p !
@mad-nuts can you do me a favour and rebase w/ the latest master and
force-push up to your branch, for one more CI / cordova-qa run, please
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-contacts/pull/146
Oh, and @mad-nuts have you signed an Apache ICLA? (individual contributor
license agreement)
---
If your project is set up for it, you can reply to this email and have your
reply
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-contacts/pull/81
Haha I was only semi-serious about the review but thanks anyways! I think I
will merge this in...
---
If your project is set up for it, you can reply to this email and have your
Github user filmaj commented on a diff in the pull request:
https://github.com/apache/cordova-docs/pull/685#discussion_r104516393
--- Diff: www/_posts/2017-03-06-plugins-release.md ---
@@ -0,0 +1,199 @@
+---
+layout: post
+author:
+name: Steve Gill
+url
GitHub user filmaj opened a pull request:
https://github.com/apache/cordova-android/pull/366
CB-12546: More robust support spawning the emulator with newer Android SDK
### Platforms affected
Android
### What does this PR do?
Two things related to
Github user filmaj commented on the issue:
https://github.com/apache/cordova-android/pull/366
Ah, I forgot to ping you: please review @infil00p
---
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
GitHub user filmaj opened a pull request:
https://github.com/apache/cordova-lib/pull/525
CB-12557: superspawn now returns both stdout and stderr in reject handler
Please review @stevengill and @audreyso.
### Platforms affected
All.
### What does this PR
Github user filmaj commented on the issue:
https://github.com/apache/cordova-lib/pull/525
@stevengill patch version bump is fine for this, yeah?
---
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
Github user filmaj commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/367#discussion_r105241573
--- Diff: bin/templates/cordova/lib/builders/GradleBuilder.js ---
@@ -65,6 +65,20 @@ GradleBuilder.prototype.getArgs = function(cmd, opts
Github user filmaj commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/367#discussion_r105241092
--- Diff: bin/lib/check_reqs.js ---
@@ -78,21 +79,46 @@ module.exports.check_ant = function
Github user filmaj commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/367#discussion_r105241332
--- Diff: bin/lib/check_reqs.js ---
@@ -78,21 +79,46 @@ module.exports.check_ant = function
Github user filmaj commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/367#discussion_r105240916
--- Diff: bin/lib/check_reqs.js ---
@@ -78,21 +79,46 @@ module.exports.check_ant = function
Github user filmaj commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/367#discussion_r105246727
--- Diff: bin/templates/cordova/lib/builders/GradleBuilder.js ---
@@ -65,6 +65,20 @@ GradleBuilder.prototype.getArgs = function(cmd, opts
Github user filmaj commented on the issue:
https://github.com/apache/cordova-android/pull/366
@infil00p OK, I'll try to get to the bottom of that. I'll ping you once
more when I do.
---
If your project is set up for it, you can reply to this email and have your
reply
Github user filmaj commented on the issue:
https://github.com/apache/cordova-android/pull/366
> Related: I've been burned by this more times than I can count, maybe we
should get rid of this check?
Nah, I think it's a good check. It runs fast and it enforces
Github user filmaj commented on the issue:
https://github.com/apache/cordova-android/pull/366
Alright @infil00p, fixed that up. Please review, thanks!
---
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
Github user filmaj commented on the issue:
https://github.com/apache/cordova-android/pull/366
OK I've rebased and tweaked based on some upcoming changes I'm prepping for
CB-12554.
@infil00p can you review one more time, please?
---
If your project is set up for i
Github user filmaj commented on the issue:
https://github.com/apache/cordova-android/pull/366
I see. I will take a look at it on my Windows VM.
---
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
Github user filmaj commented on the issue:
https://github.com/apache/cordova-android/pull/366
OK, I had to add more robust support for automatically adding the location
of the `avdmanager` and `sdkmanager` binaries to the PATH, and I broke the
tests once more. At this point, I&#
Github user filmaj commented on the issue:
https://github.com/apache/cordova-android/pull/366
I'm actually going to close this pull request and re-open once I have added
tests and thoroughly tested on both Windows and Mac.
---
If your project is set up for it, you can reply to
Github user filmaj closed the pull request at:
https://github.com/apache/cordova-android/pull/366
---
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 wishes so, or if the
Github user filmaj commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/367#discussion_r106043706
--- Diff: bin/lib/check_reqs.js ---
@@ -246,7 +273,11 @@ module.exports.check_android_target =
function(originalError) {
// Google Inc
Github user filmaj commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/367#discussion_r106050913
--- Diff: bin/lib/check_reqs.js ---
@@ -246,7 +273,11 @@ module.exports.check_android_target =
function(originalError) {
// Google Inc
GitHub user filmaj opened a pull request:
https://github.com/apache/cordova-android/pull/369
Updated CLI scripts to support Android SDK Tools 25.3.1
### Platforms affected
Android
### What does this PR do?
Adds support for using the Android SDK
Github user filmaj commented on the issue:
https://github.com/apache/cordova-android/pull/369
@dpogue thanks for checking! So I think `android_sdk_version` failing in
this case is kind of expected, as that script, historically, did not do any
environment checking, and I tried keeping
Github user filmaj commented on the issue:
https://github.com/apache/cordova-android/pull/369
Oh yes, and ping @alsorokin - not sure if the changes to the android CLI
scripts affect the CI in any way? But in any case, probably worth getting your
eyes on this change too :)
---
If
Github user filmaj commented on the issue:
https://github.com/apache/cordova-android/pull/369
@shazron I think that's expected and will happen if you also try it with
the `master` branch. It is because `android_sdk_version` does not leverage
`check_reqs`, which will modify your
GitHub user filmaj opened a pull request:
https://github.com/apache/cordova-docs/pull/686
CB-12559: documentation updates for new Android SDK Tools
Ping @infil00p, @shazron, @dpogue, @purplecabbage for review, please and
thank you.
Please do not merge this until
Github user filmaj commented on the issue:
https://github.com/apache/cordova-android/pull/369
I've posted a DISCUSS for removal of the `android_sdk_version` script here:
http://markmail.org/message/k4oysup6lkfzk4o2
Any opposition to me merging it in? I am hesitant to
Github user filmaj commented on a diff in the pull request:
https://github.com/apache/cordova-docs/pull/686#discussion_r106955951
--- Diff: www/docs/en/dev/guide/platforms/android/index.md ---
@@ -62,19 +63,20 @@ they dip below 5% on Google's
### Java Developmen
Github user filmaj commented on the issue:
https://github.com/apache/cordova-android/pull/369
Rebased on latest master, will wait for appveyor to pass before merging.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If
Github user filmaj commented on the issue:
https://github.com/apache/cordova-docs/pull/686
apache/cordova-android#369 has now landed, so assuming this is ready to be
merged assuming there are no further issues.
---
If your project is set up for it, you can reply to this email and
GitHub user filmaj opened a pull request:
https://github.com/apache/cordova-plugin-screen-orientation/pull/14
Add manual tests
Please review @purplecabbage and @maverickmishra.
### Platforms affected
All.
### What does this PR do?
Adds
Github user filmaj commented on the issue:
https://github.com/apache/cordova-android/pull/370
For reference, you can get canary and beta versions of the SDK here:
https://developer.android.com/studio/preview/index.html
A summary of using the canary and regular versions of the
Github user filmaj commented on the issue:
https://github.com/apache/cordova-android/pull/370
Results of my testing with this.
# Mac
Side-by-side installations of Android Studio + Android Studio Preview:
```
â l /Applications/Android*
/Applications
Github user filmaj commented on the issue:
https://github.com/apache/cordova-cli/pull/272
I'm confused about this command. Perhaps this is not the right place to
discuss, and feel free to link me to relevant discussion or documentation about
the command, but this command just
Github user filmaj commented on a diff in the pull request:
https://github.com/apache/cordova-cli/pull/267#discussion_r107738404
--- Diff: doc/cordova.txt ---
@@ -6,6 +6,8 @@ Global Commands
create . Create a project
help
Github user filmaj commented on a diff in the pull request:
https://github.com/apache/cordova-cli/pull/267#discussion_r107737934
--- Diff: doc/config.txt ---
@@ -0,0 +1,22 @@
+Synopsis
+
+cordova-cli config [options]
+
+
+The config command can be
Github user filmaj commented on the issue:
https://github.com/apache/cordova-cli/pull/272
Thanks for pointing me in the right direction @audreyso ! Left some
comments there, and if any changes arise from that PR, I would recommend
reflecting the wording changes from that PR in this
Github user filmaj commented on the issue:
https://github.com/apache/cordova-android/pull/370
Thanks for cleaning up the logging. However, this point still stands:
> So, it doesn't pick the preview version, just the regular Android Studio
version - unlik
Github user filmaj commented on the issue:
https://github.com/apache/cordova-android/pull/370
Got it, sounds good then.
---
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
Github user filmaj commented on the issue:
https://github.com/apache/cordova-android/pull/370
Thanks for letting us know, @PierBover! We will keep our eyes peeled for
your report.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user filmaj commented on a diff in the pull request:
https://github.com/apache/cordova-docs/pull/691#discussion_r109230732
--- Diff: www/_posts/2017-03-31-android-release.md ---
@@ -0,0 +1,38 @@
+---
+layout: post
+author:
+name: Steve Gill
+url
Github user filmaj commented on a diff in the pull request:
https://github.com/apache/cordova-docs/pull/691#discussion_r109231097
--- Diff: www/_posts/2017-03-31-android-release.md ---
@@ -0,0 +1,38 @@
+---
+layout: post
+author:
+name: Steve Gill
+url
Github user filmaj commented on the issue:
https://github.com/apache/cordova-docs/pull/691
@infil00p's makes a great point here:
> We should explicitly state here that we now require either Gradle or
Android Studio to be installed, since our old configuration didn&#
GitHub user filmaj opened a pull request:
https://github.com/apache/cordova-android/pull/373
Support for SDK Tools v26, simplified target parsing and preference for
using newer `sdkmanager` and `avdmanager` commands
Please review and test @infil00p, @dpogue, @shazron
Github user filmaj closed the pull request at:
https://github.com/apache/cordova-android/pull/373
---
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 wishes so, or if the
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-test-framework/pull/23
Thanks for the pull request! Merged in.
---
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
Github user filmaj commented on the issue:
https://github.com/apache/cordova-docs/pull/692
Thanks for the pull request! Merged in.
---
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 filmaj commented on the issue:
https://github.com/apache/cordova-plugin-camera/pull/260
Good to merge this?
---
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
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-camera/pull/257
I'm not sure we can merge this PR in as it relies on a non-Apache plugin.
We would have to move that file provider plugin under the Cordova umbrella.
Thoughts @stevengill ?
-
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-camera/pull/256
This PR looks to be commenting out a whole bunch of code and adding
incorrect properties (such as setting aspect to width).
We won't be accepting this.
---
If your proje
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-battery-status/pull/43
This looks fine to me as well. Will close this.
---
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
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-vibration/pull/52
Thanks!
---
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 wishes
GitHub user filmaj opened a pull request:
https://github.com/apache/cordova-plugin-vibration/pull/53
CB-12695: fix jshint / double-varing
### Platforms affected
All - JS
### What does this PR do?
Fixes `npm test` failure due to double-definition of
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-wkwebview-engine/pull/33
FYI I've filed [CB-12696](https://issues.apache.org/jira/browse/CB-12696)
to link to this.
---
If your project is set up for it, you can reply to this email and have
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-contacts/pull/142
Let there be 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 feature
enabled
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-contacts/pull/149
Reporter has not responded in the JIRA thread. How should we proceed
@alsorokin ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-media-capture/pull/76
Ping @purplecabbage @shazron for a quick review. Is having these microphone
images something we would want?
I am concerned that the images and configuration for this
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-media-capture/pull/73
I don't think we will be accepting this. Only a single-platform change,
setting to an arbitrary bit rate. At a minimum, this would have to be a part of
the spec acros
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-device/pull/62
@infil00p should we close this?
---
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 filmaj commented on the issue:
https://github.com/apache/cordova-plugin-dialogs/pull/92
No answer from reporter, will close this down.
---
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
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-statusbar/pull/78
Two PRs for the same feature, wow! Thanks you two.
Have either of you signed an ICLA?
---
If your project is set up for it, you can reply to this email and have your
reply
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-media-capture/pull/76
Do we need an ICLA signed for this PR?
---
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
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-media/pull/138
This good to merge?
---
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
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-media/pull/135
This looks like a completely different plugin. Closing.
---
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
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-media/pull/133
Please stop using pull requests to ask for help. Closing.
---
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
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-media/pull/132
@shazron @jcesarmobile How does this relate to #134 ?
---
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
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-media/pull/130
@ghenry22 any updates on your testing?
On the one side, I am not super keen on adding undocumented functionality,
on the other, this already exists in iOS
---
If your
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-splashscreen/pull/125
Ping @infil00p for a review.
@Lazza have you signed an ICLA for Apache?
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-geolocation/pull/87
Ping @shazron for a review.
@avishekcode have you signed an Apache ICLA?
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-geolocation/pull/88
Please stop using pull requests to ask questions. Feel free to use Slack or
contact us on our mailing lists: cordova.apache.org/contact
Closing.
---
If your project is
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-file-transfer/pull/174
Hi @Lemon-King, thanks for the PR. Can you explain your use case a bit more?
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-file-transfer/pull/177
I think we should try to write a test for this, if possible.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-file/pull/199
Let there be 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 feature
enabled and
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-file-transfer/pull/174
I see, so any custom handling of cookies via CookieManager are being
appended over and on top of whatever the WebView is handling. I guess if there
are conflicts between the
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-statusbar/pull/78
Cool, so we can fall back on your PR should @urmx be unable to. What about
you, @urmx ?
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-file/pull/199
As long as @kerrishotts still approves, I think we can pull it in.
@kerrishotts do you think we need @LightZam to sign an ICLA for this?
Worth noting that the [File API w3c spec
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-geolocation/pull/87
@avishekcode in general it is required, I believe there is some wiggle room
for trivial contributions, but in this case, I think the changes are
significant enough that we'll
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-statusbar/pull/78
Excellent, thanks for signing the ICLA!
Ping @hollyschinsky - can you check what @urmx and @tobiasviehweger mention
here with respect to the blacktranslucent/blackopaque bits
1 - 100 of 466 matches
Mail list logo
