cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From filmaj <...@git.apache.org>
Subject [GitHub] cordova-plugin-file-transfer issue #150: CB-11534 This plugin should support...
Date Tue, 18 Jul 2017 23:31:04 GMT
Github user filmaj commented on the issue:

    https://github.com/apache/cordova-plugin-file-transfer/pull/150
  
    Alright, I took a look at this again after an extended break and I have more comments,
and want to bring more information to the community about the future of this plugin as well.
    
    First, and I think most important, we need to be clear about the future of this plugin.
A couple of months back, Cordova developers did an [audit of the core cordova plugins](https://issues.apache.org/jira/browse/CB-12708),
and one result was the decision to [sunset the File Transfer plugin](https://issues.apache.org/jira/browse/CB-12711)
in [favour of the increasingly-available XHR2 APIs](http://caniuse.com/#feat=xhr2). The XHR2
(now simply merged into the core XHR spec) features that are relevant to File Transfer plugin
users are the ability to upload and to receive progress events - giving us exact feature parity
for what File Transfer provides to users today. The benefit to sunsetting this plugin is that
there would be less stuff to maintain for Cordova as XHR2 is already widely available on most
mobile platform. With that in mind, that makes me reluctant to make many changes to this plugin
as it's essentially makework.
    
    Second, looking at the specific code changes in here, I started wondering, why execute
the permission requesting inside this plugin? This plugin already depends on the core cordova
File plugin, which does have support for Marshmallow permission model - why duplicate that?
Turns out it is because the read/write methods of File Transfer, on the native Android side,
are completely recreated/reproduced! I think a "proper" change to FT to support this would
be to leverage [cordova-plugin-file's `FileUtils` class and call its `execute` method, passing
in the right arguments to trigger the `write` action](https://github.com/apache/cordova-plugin-file/blob/master/src/android/FileUtils.java#L351-L369).
Similarly, for reading files, the same could be done, using one of the [plethora of `readAs*`
methods available in cordova-plugin-file's `FileUtils.java` class](https://github.com/apache/cordova-plugin-file/blob/master/src/android/FileUtils.java#L310-L349).
The benefits I see to using this 
 approach:
    1. no need to rewrite permission handling code - cordova-plugin-file's native Android
code already does that.
    2. we could completely remove all file-reading and file-writing native Android code inside
cordova-plugin-file-transfer, as we'd defer all of that work to the File plugin.
    
    All that being said, once Cordova sunsets this plugin, the plugin code will be open and
available to the community. The community could manage this code as it is however it seems
right to them. While the patch here is not "perfect", it works, and that is more valuable
to a lot of people.
    
    So, I think I won't take any action on this PR. I am sorry that my previous review and
comments in here made it seem like this PR would make its way into the mainline. Things have
changed, and this plugin's window of life is 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 does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


Mime
View raw message