Skip to content
This repository has been archived by the owner on Mar 16, 2019. It is now read-only.

My proposed 0.10.9 changes #489

Merged
merged 46 commits into from
Aug 31, 2017
Merged

My proposed 0.10.9 changes #489

merged 46 commits into from
Aug 31, 2017

Conversation

lll000111
Copy link
Contributor

@lll000111 lll000111 commented Aug 16, 2017

As discussed in the RNFB-dev issues.

I have no idea how I managed to create this commit an hour ago: https://github.com/lll000111/react-native-fetch-blob/commit/c2803a1cb2cf977ec824674912d3154aed261512 I was cherry-picking and git said there is one unsaved change that I had to commit, so I did a commit and I ended up with this commit with an author somebody else and the date "14 days ago". git is weird. I hope I didn't mess anything up. I compared with another copy of the repo I had on disk and it seemed okay, apart from the strange meta data of this one commit.

I only tested on Android (emulator)

grylance and others added 27 commits August 16, 2017 16:15
…res that stream.write() returns a promise?"
IMPORTANT: I wrote the iOS code BLIND (not even syntax highlighting) - this needs to be tested.

- Two or three methods that used callbacks to return results were changed to RN promises
- All methods using promises now use a Unix like code string for the first parameter, e.g. "ENOENT" for "File does not exist" (http://www.alorelang.org/doc/errno.html). The React Native bridge code itself uses this schema: it inserts "EUNSPECIFIED" when the "code" it gets back from Android/iOS code is undefined (null, nil). The RN bridge assigns the code (or "EUNSPECIFIED") to the "code" property of the error object it returns to Javascript, following the node.js example (the "code" property is not part of "standard" Javascript Error objects)
- Important errors like "No such file" are reported (instead of a general error), using the code property.
- I added a few extra error checks that the IDE suggested, mostly for Android (for which I have an IDE), if it seemed important I tried to do the same for teh iOS equivalent function
- I followed IDE suggestions on some of the Java code, like making fields private
- RNFetchBlobFS.java removeSession(): Added reporting of all failures to delete - IS THIS DESIRABLE (or do we not care)?
- readStream: The same schema is used for the emitted events when they are error events
- iOS: added an import for the crypto-digest headers - they are needed for the hash() function submitted in an earlier commit
- Fixed a link in the README.md - unfortunately the anchor-links change whenever even one character of the linked headline in the Wiki page changes
…d left in when I converted mkdir to promises (low-level code)
…ile OR folder); return "true" (boolean) on success (failure is rejected promise) - it is not possibel to return "undefined" from a React Native promise from Java
…s/native-modules-android.html#argument-types "long" is not possible as argument type of an exported RN native module function
…o code is returned (should not happen, actually)
…normalized schema (#460); Open a new question about behavior on ENOENT (#491)
… mandatory even for single-statement blocks)
2) I had gone overboard with the "@[..]" in the ios code, making some error strings arrays
3) fix typos: rename all ENODIR => ENOTDIR
@lll000111 lll000111 requested a review from wkh237 August 22, 2017 06:22
@wkh237
Copy link
Owner

wkh237 commented Aug 23, 2017

@lll000111 , thanks for the great job 👍 I'll take some time review and test on the PR this weekend.

@wkh237
Copy link
Owner

wkh237 commented Aug 27, 2017

@lll000111 , I've just pulled this branch to my local environment and fixed several syntax errors in IOS, you can see my changes on branch review-489. I will run tests later on it later 👍

Copy link
Owner

@wkh237 wkh237 left a comment

Choose a reason for hiding this comment

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

@lll000111 , I've done the tests and also fixed some errors on both Android and IOS, please merge the branch review-489 then we can just merge this back to 0.10.9 🎉

@lll000111
Copy link
Contributor Author

I hope I did this right, you better check my new commit history because me and git are not the very best of friends: https://github.com/lll000111/react-native-fetch-blob/commits/0.10.9

@wkh237 wkh237 merged commit 7fa5761 into wkh237:0.10.9 Aug 31, 2017
danielsuo added a commit to danielsuo/react-native-fetch-blob that referenced this pull request Feb 13, 2018
* upstream/0.10.9:
  Fixed problem with type casting (wkh237#513)
  My proposed 0.10.9 changes (wkh237#489)
  wkh237#268 Cancelled task should not trigger `then` promise function
  Add ability to cancel android DownloadManager fetches (wkh237#502)
  Fix iOS initialization race condition (wkh237#499)
  prevent UIApplication methods from being called on background thread (wkh237#486)
  Implemenet fs.hash() -- wkh237#439 "Feature: Calculate file hash" (wkh237#476)
  I forgot one error string for the Android readFile() method (wkh237#475)
  Fix for wkh237#467 (wkh237#472)
  Fix for issue wkh237#468, wkh237#461, wkh237#460 and minor cleanup (wkh237#469)
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