Jump to conversation
Unresolved conversations (2)
@laurent22 laurent22 Jun 1, 2020
Also make it `logger().debug` please
Outdated
ReactNativeClient/lib/onedrive-api.js
@laurent22 laurent22 Jun 1, 2020
I don't think we should log this as that would be one statement for each chunk. Or maybe make it `logger().debug` so that it doesn't appear in production.
Outdated
ReactNativeClient/lib/onedrive-api.js
Resolved conversations (10)
@laurent22 laurent22 Jun 1, 2020
I expect some of the code above can throw exceptions, in particular `uploadChunk()`, and in this case the handle will remain open. So probably should use something like a try/catch/finally block and close the handle in `finally` (and re-throw errors in the catch block)
Outdated
ReactNativeClient/lib/onedrive-api.js
@laurent22 laurent22 Jun 1, 2020
Please make a copy of the options if you are going to modify them: `options = Object.assign({}, options)`
Outdated
ReactNativeClient/lib/shim.js
@laurent22 laurent22 Jun 1, 2020
Please move this method directly in OneDriveApi as it is useful only there.
Outdated
ReactNativeClient/lib/shim.js
@laurent22 laurent22 Jun 1, 2020
I think these two properties are not necessary as you can derive this info from the path (if it contains "createUploadSession", it needs `uploadBigFile()`. And ideally the file size should be retrieved where it's needed, directly in `uploadBigFile()`.
Outdated
...iveClient/lib/file-api-driver-onedrive.js
@laurent22 laurent22 Jun 1, 2020
`===`
Outdated
ReactNativeClient/lib/onedrive-api.js
@laurent22 laurent22 Jun 1, 2020
Please set it as `4*1024*1024`
Outdated
...iveClient/lib/file-api-driver-onedrive.js
@laurent22 laurent22 Jun 1, 2020
Please set it as `7.5 * 1024 * 1024` for clarity
Outdated
ReactNativeClient/lib/onedrive-api.js
@laurent22 laurent22 Jun 1, 2020
Please use strict equality `===`
Outdated
ReactNativeClient/lib/onedrive-api.js
@laurent22 laurent22 Jun 1, 2020
Same, you don't need to create a new method, you can simply use the existing `stat()` method.
Outdated
ReactNativeClient/lib/fs-driver-rn.js
@laurent22 laurent22 Jun 1, 2020
Please use the `stat()` method for this, and you can then get the size in the `size` property. As the code in this file needs to be duplicated for each platform, we try to keep it as lean as possible.
Outdated
ReactNativeClient/lib/fs-driver-node.js