Add sync progress #991
Add sync progress #991
Conversation
|
WalkthroughThis update introduces a new interface Changes
Uplevel your code reviews with Entelligence.AI ProEntelligence.AI ProIf you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, Entelligence.AI Pro is free for open source projects. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Files selected (3)
- app/lib/pages/memories/page.dart (1)
- app/lib/providers/memory_provider.dart (3)
- app/lib/services/wals.dart (4)
Review comments generated (9)
- Review: 6
- LGTM: 3
Tips
Chat with Entelligence.AI Bot (@Entelligence.AI)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@Entelligence.AIin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@Entelligence.AI: ignoreanywhere in the PR description to pause further reviews from the bot.
| child: memoryProvider.walsSyncedProgress == 0.0 | ||
| ? Text( | ||
| "You have ${memoryProvider.missingWals.map((val) => val.seconds).reduce((a, b) => a + b)}s stereo localy, sync now?") | ||
| : Text("${(memoryProvider.walsSyncedProgress * 100).toInt()}%"), |
There was a problem hiding this comment.
The code assumes that memoryProvider.missingWals is not empty. If it's empty, calling reduce on it will throw an error. You should add a check to ensure missingWals is not empty before trying to reduce it.
- "You have ${memoryProvider.missingWals.map((val) => val.seconds).reduce((a, b) => a + b)}s stereo localy, sync now?")
+ "You have ${memoryProvider.missingWals.isNotEmpty ? memoryProvider.missingWals.map((val) => val.seconds).reduce((a, b) => a + b) : 0}s stereo localy, sync now?")Also, there's a typo in the word 'locally'. It should be corrected as well.
- "You have ${memoryProvider.missingWals.isNotEmpty ? memoryProvider.missingWals.map((val) => val.seconds).reduce((a, b) => a + b) : 0}s stereo localy, sync now?")
+ "You have ${memoryProvider.missingWals.isNotEmpty ? memoryProvider.missingWals.map((val) => val.seconds).reduce((a, b) => a + b) : 0}s stereo locally, sync now?")| @@ -11,7 +11,7 @@ import 'package:friend_private/services/wals.dart'; | |||
| import 'package:friend_private/utils/analytics/mixpanel.dart'; | |||
| import 'package:friend_private/utils/features/calendar.dart'; | |||
|
|
|||
| class MemoryProvider extends ChangeNotifier implements IWalServiceListener { | |||
| class MemoryProvider extends ChangeNotifier implements IWalServiceListener, IWalSyncProgressListener { | |||
There was a problem hiding this comment.
The class MemoryProvider now implements the IWalSyncProgressListener interface. Ensure that all methods from this interface are properly implemented in this class.
- class MemoryProvider extends ChangeNotifier implements IWalServiceListener {
+ class MemoryProvider extends ChangeNotifier implements IWalServiceListener, IWalSyncProgressListener {| _walsSyncedProgress = 0.0; | ||
| _wal.syncAll(progress: this); | ||
| } | ||
|
|
||
| @override | ||
| void onWalSyncedProgress(double percentage) { | ||
| _walsSyncedProgress = percentage; |
There was a problem hiding this comment.
The syncWals method has been updated to reset _walsSyncedProgress to 0 before starting synchronization and to use the this object as a progress listener. The onWalSyncedProgress method is used to update _walsSyncedProgress. Make sure that the syncAll method of _wal correctly handles the progress parameter and that it calls onWalSyncedProgress at appropriate times during the synchronization process.
- _wal.syncAll();
+ _walsSyncedProgress = 0.0;
+ _wal.syncAll(progress: this);+ @override
+ void onWalSyncedProgress(double percentage) {
+ _walsSyncedProgress = percentage;
+ }| @@ -108,7 +108,7 @@ class Wal { | |||
| } | |||
|
|
|||
| abstract class IWalSyncProgressListener { | |||
| void onWalSyncedProgress(Wal wal, Float percentage); | |||
| void onWalSyncedProgress(double percentage); // 0..1 | |||
There was a problem hiding this comment.
The method signature for onWalSyncedProgress has been changed to accept a double value representing the percentage of progress. This is a breaking change if there are any existing implementations of IWalSyncProgressListener. Make sure all implementations are updated accordingly.
- void onWalSyncedProgress(Wal wal, Float percentage);
+ void onWalSyncedProgress(double percentage); // 0..1| // Clean synced wal | ||
| for (var i = _wals.length - 1; i >= 0; i--) { | ||
| if (_wals[i].status == WalStatus.synced) { | ||
| await _deleteWal(_wals[i]); | ||
| } | ||
| } |
There was a problem hiding this comment.
This block of code deletes all Wals that have been synced. However, it's not clear whether _deleteWal(_wals[i]) is an asynchronous operation that could potentially fail. If it does fail, the error is not being handled here which could lead to inconsistencies in the state of your application. Consider adding error handling around this operation.
- await _deleteWal(_wals[i]);
+ try {
+ await _deleteWal(_wals[i]);
+ } catch (e) {
+ // Handle or log error
+ }| _wals[j].status = WalStatus.synced; | ||
|
|
||
| // Send | ||
| for (var sub in _subscriptions.values) { | ||
| sub.onWalSynced(wal); | ||
| sub.onWalSynced(_wals[j]); | ||
| } |
There was a problem hiding this comment.
This block notifies all subscribers that a Wal has been synced. However, it doesn't handle potential errors that might occur during the notification process. If any subscriber throws an error, it will stop the execution and other subscribers might not get notified. Consider adding error handling to ensure all subscribers are notified even if one fails.
- sub.onWalSynced(_wals[j]);
+ try {
+ sub.onWalSynced(_wals[j]);
+ } catch (e) {
+ // Handle or log error
+ }
Summary by Entelligence.AI
IWalSyncProgressListenerinterface to handle progress updates more efficiently and perform cleanup tasks for synced Wals.walsSyncedProgressvalue, improving user experience during Wal synchronization.