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

no-loop-func lint warning fix #2157

Merged
merged 6 commits into from
May 30, 2022
Merged

no-loop-func lint warning fix #2157

merged 6 commits into from
May 30, 2022

Conversation

tanmoyAtb
Copy link
Contributor

We had a eslint warning about an uploadedSize variable.
Screenshot 2022-05-27 at 2 34 10 PM

This PR fixes the warning.
Alternatively I wanted to ignore the eslint rule. But this works as well 🤷

@render
Copy link

render bot commented May 27, 2022

@render
Copy link

render bot commented May 27, 2022

@render
Copy link

render bot commented May 27, 2022

@github-actions github-actions bot added the Type: Maintenance Added to issues and PRs when a change is for repository maintenance , such as CI or linter changes. label May 27, 2022
Copy link
Collaborator

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

Nice one. I think the original warning doesn't make sense though, the warning acts as if the for loop wasn't waiting for the encryptAndUpload to be back, and the uploadedSize would be out of sync. The reality though is that the for is actually blocking because of the await, and the uploadedSize is always in sync.

Regardless, it's always better without any warning. I'd just add a comment for us to remember in the future.

@tanmoyAtb
Copy link
Contributor Author

Nice one. I think the original warning doesn't make sense though, the warning acts as if the for loop wasn't waiting for the encryptAndUpload to be back, and the uploadedSize would be out of sync. The reality though is that the for is actually blocking because of the await, and the uploadedSize is always in sync.

Regardless, it's always better without any warning. I'd just add a comment for us to remember in the future.

Absolutely agreed, wanted to ignore the rule in eslint config but decided to let it stay. Might be of use some day.

Copy link
Contributor

@juans-chainsafe juans-chainsafe left a comment

Choose a reason for hiding this comment

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

Checked locally and was fixed, nice!!

@Tbaut Tbaut merged commit f94b155 into dev May 30, 2022
@Tbaut Tbaut deleted the mnt/uploaded-size-warning branch May 30, 2022 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Maintenance Added to issues and PRs when a change is for repository maintenance , such as CI or linter changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants