remotestorage.js
remotestorage.js copied to clipboard
remotestorage
refactor sync.js
sync.js has an F on codeclimate, and is in need of documentation, refactoring, and reduction in overall complexity.
Who here knows the most about sync.js? Maybe we could start with that person documenting it as best they can as it works now?
Yes, I agree. I also had my fair share of head-scratching while trying to understand this part of the codebase. I would be happy to help improve it.
I think currently @michielbdejong is the person who understands it the most. It would be of great help if you could document it some more.
Yeah, maybe somehow splitting it into two files would help. For instance https://github.com/remotestorage/remotestorage.js/blob/master/src/sync.js#L299-L860 could be split out to something like task.js as it deals with actually executing the tasks (561 lines out of 1156), and the rest of the file deals with scheduling the tasks (maybe call that scheduling.js or something).
@michielbdejong would you be wiling to document this library, as it stands at this point, sometime soon?
@silverbucket basically what it does is six things:
- retrieving the remote version of relevant documents and folders
- add all local and remote documents together into one tree
- push local documents out if they don't exist remotely
- push local changes out to remote documents (conditionally, to avoid race conditions where both have changed)
- adopt the local version of a document to its remote version if both exist and they differ
- delete the local version of a document if it was deleted remotely
- if any get requests were waiting for remote data, resolve them once this data comes in.
It does this using requests to documents, and to folders. Whenever a folder GET comes in, it gives information about all the documents it contains (this is the markChildren function).
That's about it! :) If anything else is unclear then we can hang around after the team meeting next week, and we can go through it together.
I think he meant documenting the functions with naturaldocs like you started in other places in the last weeks (which is awesome, btw!), so that it's possible to refactor the whole thing without having to fit all code in that file in your brain at once.
Looks like a good start, though. Maybe we should just throw it in the beginning of the file?
I can't do it, because as I said it took a couple of days last time just to get our heads around it. It would be really helpful if you documented it.
ok, https://github.com/remotestorage/remotestorage.js/pull/779
@silverbucket sorry i don't have enough time to do a real function-by-function documentation describing each parameter etc, but if you have any questions or need help with stuff, i'll be available.
Maybe you guys could have a 10 minute screensharing session about this?
(Tip: you use Chrome for it you can do screensharing in hu.tt now.)
I added a second PR with more (stubs for) walk-through docs. There was already a long doc there describing all the complicated bits. Have you all read that? If not, then please read it now
Sure, i'm also available for screensharing, so we can click around together in the code. Just ping me.
Wrote a comment about it on the forums, where you brought it up, too: http://community.remotestorage.io/t/sync-and-collisions-conflicts/230/4
Would be nice if some more people would chime in over there!
ah I didn't notice it on the forums before I replied to the issue on github: https://github.com/remotestorage/remotestorage.js/issues/787
That's alright. Now that there's a separate GitHub issue, that's probably a better place to make progress with that decision anyway.