sboesen / remotely-sync

fork of remotely-save with security upgrades
Apache License 2.0
180 stars 8 forks source link

Bug fixes for syncRun and Sync on Remote #101

Closed kadisonm closed 6 months ago

kadisonm commented 6 months ago

Fixing metadata not uploading if the concurrency is 1 added an if statement to the already many nested for loops. I think that section needs a refactor so that the queue can upload at a concurrency of 1 (if that is possible). So that we can just delete the whole check for concurrency being 1.

sboesen commented 6 months ago

Really appreciate the PR!

I think that section needs a refactor so that the queue can upload at a concurrency of 1 (if that is possible).

I agree, do you think you could take a stab at making that change? If you don't have the cycles to do that happy to take that on at some point, too, we can merge as is. Just feels like a good opportunity.

kadisonm commented 6 months ago

Yep I can do that. I had a try at refactoring it before letting you know and I think I have fixed a lot of issues. I believe the for loops were finishing before some promises were resolved, which was causing the metadata to sync before it had finished syncing. Switched it all to for of loops to make sure everything is done before continuing. I'll commit soon.

kadisonm commented 6 months ago

I updated the pull request description to list the fixes I think it fixes. I need to run a couple tests first to confirm.

Edit: Can confirm it fixes the status bar issues and syncing a large vault is really satisfying to watch now.

FDwPph7REd

For the most part it fixes the status bar jittering. But I did notice it change once to something else then quickly back to the queue numbers.

sboesen commented 6 months ago

Wow, that's great news! Glad to hear. I'll merge and do my own testing & ship this release. I'll keep the issues open add comments to the issues and take the time to do more testing (have some test vaults attached to those issues that used to have issues, worth double checking they are gone).

Did the status bar issues go away with concurrency > 1? Or for concurrency == 1?

Thank you again for the PR, greatly appreciated!