Data sync merge - Githubissues

status-im / status-console-client

Status messaging console user interface

Mozilla Public License 2.0

10 stars 2 forks source link

Data sync merge #86

Closed adambabik closed 5 years ago

adambabik commented 5 years ago

Changes:

DataSyncWhisperAdapter was created in adapter/. It conforms to protocol.Protocol just like other adapters. It handles incoming packets in Whisper messages by passing them to an object implementing PacketHandler interface.
There is a new package protocol/datasync which contains DataSyncNodeTransport which implements an interface required by mvds.Node.
Additionally, there is protocol/datasync/peer which contains data sync peer utility functions. It's in a separate package to avoid cyclic import (should probably be refactored and some code moved to mvds).

To run data sync use -ds flag.

decanus commented 5 years ago

@adambabik should we not merge this into data-sync rather than master? Other than that it looks good to me, I would merge it in the other branch as there are a lot of features relying on that one.

adambabik commented 5 years ago

should we not merge this into data-sync rather than master?

@decanus no, I would prefer to keep just one branch. Initially, I suggested creating data-sync branch just because we need to refactor the code to support something like data sync. But as it happened in #84, I was able to move code from data-sync to this PR and merge it to master without affecting anything else or duplicating code.

For other PRs, I suggest we merge this one and then rebase these other branches on master. I know that there will be many conflicts but I don't see other way... Having just one branch is much more beneficial than one time conflict resolution.

decanus commented 5 years ago

@adambabik ok sounds good