-
Notifications
You must be signed in to change notification settings - Fork 7
capitalize more shorthand names #10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
what will this break? we can't be breaking existing KBFS clients. Will this break those? |
|
Not from where I see. When we update protocols in client, it breaks Go code, which should be easy to fix from compiler errors. Without updated vendor in KBFS, it would remain the same, and since codec doesn't change, it wouldn't change message on-the-wire. Then we can update vendor in KBFS. It seems to me that nothing will break; we'll just be updating names in Go code. Does that sound correct? |
|
In general, we further require that all local keybase processes work with other keybase processes from earlier versions, since restarts don't happen all at once. Seems like this change will break backwards/forwards compatibility between kbfs and the keybase service. |
|
I don't think this change is worth the risk. The risk is that our software will break in weird ways in the wild. The upside is extremely limited. Users won't benefit at all. Our team will see it as slightly more consistent. |
|
Between KBFS and service it's RPC, which is done using the so-called over-the-wire encoding right? The binary is always one file and when updated, it's the entire process |
|
As I said above, we've had the requirement that even local upgrades don't break backwards compatibility. We've definitely had issues in the past with upgrades that did. |
|
I still don't under how this breaks backward compatibility. It doesn't change message on-the-wire, thus RPCs should still work like before ... |
|
Ah. I see. You are right.
…On Fri, Dec 2, 2016 at 11:51 PM Song Gao ***@***.***> wrote:
I still don't under how this breaks backward compatibility. It doesn't
change message on-the-wire, thus RPCs should still work like before ...
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#10 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA05_6nMEqb_9KOOfq-CfCqE-qE-fDEOks5rEPU_gaJpZM4LDMCk>
.
|
|
Ok thanks for settting me straight. We can do this but it is a big change on the go side. Is it worth the headache? The status quo doesn't bother me much. But I agree it won't break wire compatibility. |
|
No problem; Thanks Max! There are renaming and refactoring tools from |
r? @maxtaco @patrickxb Thanks!
This should make
TLFMessage(as opposed toTlfMessage) possible