Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[file-system] Fix UploadProgressData field name typo #20804

Merged

Conversation

gabrieldonadel
Copy link
Member

@gabrieldonadel gabrieldonadel commented Jan 12, 2023

Why

UploadProgressData totalByteSent key should be totalBytesSent to be consistent with other FIleSystem properties like totalBytesExpectedToSend, totalBytesWritten and totalBytesExpectedToWrite.

Closes ENG-6559

Follow up PR updating docs -> #20810

How

This updates all native references of totalByteSent to totalBytesSent and adds a getter totalByteSent property to UploadProgressData events so users get a deprecation warning when accessing totalByteSent

Test Plan

Added a new Download & Upload file button to NCL so we can properly test this

Screen.Recording.2023-01-12.at.09.35.22.mov

Checklist

@linear
Copy link

linear bot commented Jan 12, 2023

ENG-6559 FileSystem UploadProgressData field name typo

totalByteSent should be totalBytesSent to be consistent with:

  • totalBytesExpectedToSend
  • totalBytesWritten
  • totalBytesExpectedToWrite

Also needs to be changed on native side, eg:

@"totalByteSent": @(totalBytesSent),
@"totalBytesExpectedToSend": @(totalBytesExpectedToSend),

@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Jan 12, 2023
@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Jan 12, 2023
@gabrieldonadel gabrieldonadel marked this pull request as ready for review January 12, 2023 13:34
Copy link
Member

@tsapeta tsapeta left a comment

Choose a reason for hiding this comment

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

Cool, thanks for fixing this! 🙏

@@ -4,6 +4,8 @@

### 🛠 Breaking changes

- Rename `UploadProgressData` `totalByteSent` field to `totalBytesSent`. ([#20804](https://github.com/expo/expo/pull/20804) by [@gabrieldonadel](https://github.com/gabrieldonadel))
Copy link
Member

Choose a reason for hiding this comment

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

Let's move that to Others category. It's not a breaking change as long as it's backwards compatible 😉

Copy link
Member Author

Choose a reason for hiding this comment

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

Opss my bad, updated

@gabrieldonadel gabrieldonadel merged commit 4ee48f9 into main Jan 12, 2023
@gabrieldonadel gabrieldonadel deleted the @gabrieldonadel/fix-upload-progress-data-field-name-typo branch January 12, 2023 23:58
gabrieldonadel added a commit that referenced this pull request Jan 13, 2023
<!-- disable:changelog-checks -->
# Why

Follow-up PR for #20804

Closes ENG-6559

# How

`et generate-docs-api-data --packageName expo-file-system`

# Test Plan

Changes have been tested by running docs locally.

![image](https://user-images.githubusercontent.com/11707729/212104429-82698e24-8db0-49b5-8cd7-ae08869ea2f1.png)


# Checklist

<!--
Please check the appropriate items below if they apply to your diff.
This is required for changes to Expo modules.
-->

- [x] Documentation is up to date to reflect these changes (eg:
https://docs.expo.dev and README.md).
- [ ] Conforms with the [Documentation Writing Style
Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
- [ ] This diff will work correctly for `expo prebuild` & EAS Build (eg:
updated a module plugin).
byCedric pushed a commit that referenced this pull request Jan 17, 2023
# Why

`UploadProgressData` `totalByteSent` key should be `totalBytesSent` to
be consistent with other FIleSystem properties like
`totalBytesExpectedToSend`, `totalBytesWritten` and
`totalBytesExpectedToWrite`.


Closes ENG-6559

Follow up PR updating docs -> #20810

# How

This updates all native references of `totalByteSent` to
`totalBytesSent` and adds a getter `totalByteSent` property to
`UploadProgressData` events so users get a deprecation warning when
accessing `totalByteSent`

# Test Plan

Added a new `Download & Upload file` button to NCL so we can properly
test this


https://user-images.githubusercontent.com/11707729/212067976-a4bdd509-8937-4f27-9516-cee35ed94dca.mov



# Checklist

<!--
Please check the appropriate items below if they apply to your diff.
This is required for changes to Expo modules.
-->

- [ ] Documentation is up to date to reflect these changes (eg:
https://docs.expo.dev and README.md).
- [ ] Conforms with the [Documentation Writing Style
Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
- [ ] This diff will work correctly for `expo prebuild` & EAS Build (eg:
updated a module plugin).
byCedric pushed a commit that referenced this pull request Jan 17, 2023
<!-- disable:changelog-checks -->
# Why

Follow-up PR for #20804

Closes ENG-6559

# How

`et generate-docs-api-data --packageName expo-file-system`

# Test Plan

Changes have been tested by running docs locally.

![image](https://user-images.githubusercontent.com/11707729/212104429-82698e24-8db0-49b5-8cd7-ae08869ea2f1.png)


# Checklist

<!--
Please check the appropriate items below if they apply to your diff.
This is required for changes to Expo modules.
-->

- [x] Documentation is up to date to reflect these changes (eg:
https://docs.expo.dev and README.md).
- [ ] Conforms with the [Documentation Writing Style
Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
- [ ] This diff will work correctly for `expo prebuild` & EAS Build (eg:
updated a module plugin).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: passed checks ExpoBot has nothing to complain about
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants