feat(sync): support offline commit- [INS-4226]#7811
Conversation
CurryYangxx
changed the title
CurryYangxx
force-pushed
the
feat/offline-commit
branch
from
5a844b4 to
0c5999d
Compare
| remoteBranches = ( | ||
| remoteBranchesCache[workspaceId] || (await vcs.getRemoteBranchNames()) | ||
| ).sort(); | ||
| compare = |
There was a problem hiding this comment.
Would it make sense to split these in two try/catch?
My thought is: In case one of them fails and the other works we would at least have the correct value for one of them
There was a problem hiding this comment.
Also looking at this part and i have another question, will splitting introduce inconsistency? this seems for avoiding noise when offline, it might be great if we could differentiate different errors.
There was a problem hiding this comment.
Previously, we returned an error if any of them failed, and did not show the entrance for sync. Now, I have wrapped the logic that relies on the network in a new try/catch block so that we can get some of the sync data when the request fails or the user is offline. This allows users to use the sync features that do not depend on the network.
There was a problem hiding this comment.
Do you have some other suggestions for error handling?
There was a problem hiding this comment.
Not sure I have other thoughts on this. I feel much more confident baed on your comment so LGTM
ihexxa
left a comment
ihexxa
left a comment
There was a problem hiding this comment.
Overall lgtm and error handling might worth a bit discussion.
| remoteBranches = ( | ||
| remoteBranchesCache[workspaceId] || (await vcs.getRemoteBranchNames()) | ||
| ).sort(); | ||
| compare = |
There was a problem hiding this comment.
Also looking at this part and i have another question, will splitting introduce inconsistency? this seems for avoiding noise when offline, it might be great if we could differentiate different errors.
| remoteBranches = ( | ||
| remoteBranchesCache[workspaceId] || (await vcs.getRemoteBranchNames()) | ||
| ).sort(); | ||
| compare = |
There was a problem hiding this comment.
Not sure I have other thoughts on this. I feel much more confident baed on your comment so LGTM
CurryYangxx
force-pushed
the
feat/offline-commit
branch
from
7cf80e6 to
04a2cce
Compare
3 participants
Support offline commit for Insomnia cloud sync.