Skip to content

Conversation

lohanidamodar
Copy link
Member

@lohanidamodar lohanidamodar commented Nov 2, 2021

  • Chunked upload support on Flutter and Dart
  • Limitation: Flutter Web and Dart JS platforms will not work with files above 5 MB

@lohanidamodar lohanidamodar changed the title Large File Chunked Upload support on Flutter and Dart Large File Chunked Upload support on Flutter Nov 2, 2021
@lohanidamodar lohanidamodar marked this pull request as ready for review November 2, 2021 09:54
@lohanidamodar lohanidamodar requested a review from wess November 19, 2021 10:41
Copy link
Contributor

@wess wess left a comment

Choose a reason for hiding this comment

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

Overall looks really good, just a couple of things, mainly inconsistency in patterns. Dart's VM/AST is weird, it transverses ELSE to additional if evaluations with look a-head, so if we can return in the if without fall through to an else statement, it might save us a few cycles

params['file'] = await http.MultipartFile.fromPath(paramName, file.path!, filename: file.fileName);
res = await client.call(HttpMethod.post,
path: path, params: params, headers: headers);
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why "else" statement? Perhaps just return "await client.call..." exit quicker to shorten cycles.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hey, good catch. I've made the changes, let me know what you think.

if (size <= Client.CHUNK_SIZE) {
params['file'] = await http.MultipartFile.fromPath(paramName, file.path!, filename: file.fileName);
res = await client.call(HttpMethod.post,
path: path, params: params, headers: headers);
Copy link
Contributor

Choose a reason for hiding this comment

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

In service.dart.twig (line 70 - 76) you place each argument on a different line. I like this makes readability super clear. Here, perhaps we could follow the same pattern?

@lohanidamodar lohanidamodar requested a review from wess November 22, 2021 04:57
@lohanidamodar lohanidamodar changed the base branch from master to feat-preps-for-0.13 January 12, 2022 07:46
@lohanidamodar lohanidamodar merged commit 96735ee into feat-preps-for-0.13 Jan 30, 2022
@lohanidamodar lohanidamodar deleted the feat-large-file-dart branch January 30, 2022 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants