Add UNKNOWN values to blimp proto enums

Add UNKNOWN values to blimp proto enums

BUG=559338
Committed: https://crrev.com/d256fd0832d4772fdea88fac6d19a940d248b5c2
Cr-Commit-Position: refs/heads/master@{#361186}

[maniscalco, 2015-11-20 22:33:47]

maniscalco@chromium.org changed reviewers:
+ wez@chromium.org

[maniscalco, 2015-11-20 22:33:48]

https://codereview.chromium.org/1469503002/diff/1/blimp/common/proto/blimp_me...
File blimp/common/proto/blimp_message.proto (right):

https://codereview.chromium.org/1469503002/diff/1/blimp/common/proto/blimp_me...
blimp/common/proto/blimp_message.proto:36: UNKNOWN = 0;
Breaking the cardinal rule here, but I believe it's OK because there is nothing
"in the wild" for us to break.  Of course if we have serialized protos stashed
away somewhere (files for replay, databases, etc.).  We'd need to update those
as well.

[Wez, 2015-11-20 23:33:39]

lgtm

https://codereview.chromium.org/1469503002/diff/1/blimp/common/proto/blimp_me...
File blimp/common/proto/blimp_message.proto (right):

https://codereview.chromium.org/1469503002/diff/1/blimp/common/proto/blimp_me...
blimp/common/proto/blimp_message.proto:36: UNKNOWN = 0;
On 2015/11/20 at 22:33:48, maniscalco wrote:
> Breaking the cardinal rule here, but I believe it's OK because there is
nothing "in the wild" for us to break.  Of course if we have serialized protos
stashed away somewhere (files for replay, databases, etc.).  We'd need to update
those as well.

Agreed; we are OK w/ breaking changes to the protocol at least until we reach
the v0.5 milestone, FWIW.

nit: Is UNKNOWN the convention, or INVALID?  Fundamentally, via versioning, a
peer sending out-of-range values is misbehaving, so I'd treat that as invalid
input.

[maniscalco, 2015-11-20 23:42:21]

https://codereview.chromium.org/1469503002/diff/1/blimp/common/proto/blimp_me...
File blimp/common/proto/blimp_message.proto (right):

https://codereview.chromium.org/1469503002/diff/1/blimp/common/proto/blimp_me...
blimp/common/proto/blimp_message.proto:36: UNKNOWN = 0;
On 2015/11/20 23:33:39, Wez wrote:
> On 2015/11/20 at 22:33:48, maniscalco wrote:
> > Breaking the cardinal rule here, but I believe it's OK because there is
> nothing "in the wild" for us to break.  Of course if we have serialized protos
> stashed away somewhere (files for replay, databases, etc.).  We'd need to
update
> those as well.
> 
> Agreed; we are OK w/ breaking changes to the protocol at least until we reach
> the v0.5 milestone, FWIW.
> 
> nit: Is UNKNOWN the convention, or INVALID?  Fundamentally, via versioning, a
> peer sending out-of-range values is misbehaving, so I'd treat that as invalid
> input.

The convention I'm familiar with is to use UNKNOWN or FOO_UNKNOWN where FOO is
the name of the enum.

I understand what you mean about it being invalid in our use.  My instinct is to
stick with UNKNOWN since it's more common, but I'm happy to change it if you
feel strongly.

[maniscalco, 2015-11-23 21:02:40]

The CQ bit was checked by maniscalco@chromium.org

[maniscalco, 2015-11-23 21:02:40]

The patchset sent to the CQ was uploaded after l-g-t-m from wez@chromium.org
Link to the patchset: https://codereview.chromium.org/1469503002/#ps20001
(title: "Merge with master")

[commit-bot: I haz the power, 2015-11-23 21:07:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1469503002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1469503002/20001

[commit-bot: I haz the power, 2015-11-23 21:27:15]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-11-23 21:28:12]

Patchset 2 (id:??) landed as
https://crrev.com/d256fd0832d4772fdea88fac6d19a940d248b5c2
Cr-Commit-Position: refs/heads/master@{#361186}