Move notifications package to status-go codebase.
#692
Conversation
mandrigin
force-pushed
the
igorm/655-2
branch
2 times, most recently
from
be66daf to
2adfeee
Compare
|
looks good, but why not to drop this packages completely, right in this change? i think it was already confirmed that it is not used |
|
@dshulyak was it? I missed that discussion... |
|
I think we can drop it. |
// enable notification service
if whisperConfig.EnablePushNotification {
log.Info("Register PushNotification server")
var notificationServer notifications.NotificationServerThat is exactly where this moved code is used,
|
|
@mandrigin maybe i misunderstood smth, but i decided that this EnabledPushNotification is not used at all |
|
@adambabik can you help? |
|
if we do then we need to cover it with e2e tests asap |
|
It does not take part in sending push notifications. We can remove it and also remove the place where it is used. |
| @@ -1,36 +0,0 @@ | |||
| diff --git a/whisper/notifications/utils.go b/whisper/notifications/utils.go | |||
There was a problem hiding this comment.
We can also remove the reference to this file from the README.md
|
Shouldn't the |
|
@pombeirp yeah, that exactly what linter told me :) |
|
The same changes should be made to |
mandrigin
force-pushed
the
igorm/655-2
branch
from
611c308 to
119d778
Compare
| return nil, errors.New("notification server requires -identity file to be specified") | ||
| } | ||
|
|
||
| if whisperConfig.IdentityFile != "" { |
There was a problem hiding this comment.
IdentityFile may be useful to make sure the nodeID stays the same.
There was a problem hiding this comment.
Scratch that. Let's leave it removed. If we hit some issues it should be implemented from scratch as this should not be a part of Whisper config if used as a nodeID.
Minimum viable changes to unblock #686.
notificationspackage is moved to status-go, unnecessary parts of patches are removed.Closes #655