Persist realname for always-on clients#1180
Conversation
slingamn
left a comment
slingamn
left a comment
There was a problem hiding this comment.
This mostly looks good. Try testing a few different cases?
I forgot to tell you: can you clean up the existing code that sets realname on reattach?
irc/client.go
Outdated
| if client.realname == "" { | ||
| client.realname = realname | ||
| if client.alwaysOn { | ||
| client.markDirty(IncludeRealname) |
There was a problem hiding this comment.
This needs to happen outside the lock as well (you can use a defer for this)
There was a problem hiding this comment.
From discussion: there's actually no need to mark dirty here, since this is never called for registered clients.
irc/getters.go
Outdated
| client.stateMutex.Lock() | ||
| client.realname = realname | ||
| client.stateMutex.Unlock() | ||
| if client.alwaysOn { |
There was a problem hiding this comment.
I missed this earlier: this is technically a data race. (You can alwaysOn := client.alwaysOn up above while holding the lock.)
irc/accounts.go
Outdated
| am.server.RandomlyRename(currentClient) | ||
| } | ||
| } | ||
| if client.alwaysOn { |
|
Excellent. I'll sleep on this. |
|
Will do some testing on new changes. |
slingamn
left a comment
slingamn
left a comment
There was a problem hiding this comment.
Looks good. Would you mind squashing when you're done?
| realname, _ = tx.Get(key) | ||
| return nil | ||
| }) | ||
| if realname == "" { |
clukawski
force-pushed
the
persist-realname-alwayson
branch
from
8dfd5cb to
6f8711d
Compare
2 participants
Implements #1065. Added implementation notes to the issue.