Remove obsolete .proto fields

Remove obsolete .proto fields

BUG=713742
Review-Url: https://codereview.chromium.org/2828303002
Cr-Commit-Position: refs/heads/master@{#466299}
Committed: https://chromium.googlesource.com/chromium/src/+/b92e7bbc8617e7b0a7c0e24f800457a51c420209

[Thiemo Nagel, 2017-04-20 14:16:34]

The CQ bit was checked by tnagel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-20 14:16:43]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-20 15:02:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-20 15:02:41]

Dry run: This issue passed the CQ dry run.

[Thiemo Nagel, 2017-04-20 15:14:20]

tnagel@chromium.org changed reviewers:
+ emaxx@chromium.org

[Thiemo Nagel, 2017-04-20 15:14:21]

Hi Maksim,

are you aware of a reason why we should not remove obsolete fields?

Thanks,
Thiemo

[emaxx, 2017-04-20 15:25:04]

Maybe we should create a bug for this protos cleanup? I think we have a bunch of
unused stuff in our protobufs, and we may start with deprecating/removing them
too.


On 2017/04/20 15:14:21, Thiemo Nagel wrote:
> Hi Maksim,
> 
> are you aware of a reason why we should not remove obsolete fields?

I'm not aware of any reason to keep unused fields.
However, to prevent the old field numbers from being reused, it's better to
mark them as reserved by the means of the "reserved" statement:
https://developers.google.com/protocol-buffers/docs/proto#reserved

[Thiemo Nagel, 2017-04-20 15:47:04]

Description was changed from

==========
Remove obsolete .proto fields

BUG=none
==========

to

==========
Remove obsolete .proto fields

BUG=713742
==========

[Thiemo Nagel, 2017-04-20 15:48:02]

Patchset #2 (id:20001) has been deleted

[Thiemo Nagel, 2017-04-20 15:48:43]

> Maybe we should create a bug for this protos cleanup?

Done.

> However, to prevent the old field numbers from being reused, it's better to
> mark them as reserved by the means of the "reserved" statement:
> https://developers.google.com/protocol-buffers/docs/proto#reserved

Excellent.  Thanks a lot!

[Thiemo Nagel, 2017-04-20 15:49:06]

The CQ bit was checked by tnagel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-20 15:49:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-20 16:36:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-20 16:36:56]

Dry run: This issue passed the CQ dry run.

[emaxx, 2017-04-20 16:59:02]

LGTM.

https://codereview.chromium.org/2828303002/diff/40001/components/policy/proto...
File components/policy/proto/device_management_backend.proto (left):

https://codereview.chromium.org/2828303002/diff/40001/components/policy/proto...
components/policy/proto/device_management_backend.proto:832: repeated
InstallableLaunch app_launch_stat = 3;
Just a note: I think we should be careful with removing not deprecated
fields/messages straight away. For instance, maybe they are laid here as a
schema for the to-be-implemented communications.
Though it seems that the fields deleted in this CL are not referenced anywhere,
including the server side code base, and are not mentioned in any plans or
design docs for the future.
So I'm fine with performing this removal.

[Thiemo Nagel, 2017-04-21 09:48:27]

Thank you!

https://codereview.chromium.org/2828303002/diff/40001/components/policy/proto...
File components/policy/proto/device_management_backend.proto (left):

https://codereview.chromium.org/2828303002/diff/40001/components/policy/proto...
components/policy/proto/device_management_backend.proto:832: repeated
InstallableLaunch app_launch_stat = 3;
On 2017/04/20 16:59:02, emaxx wrote:
> Just a note: I think we should be careful with removing not deprecated
> fields/messages straight away. For instance, maybe they are laid here as a
> schema for the to-be-implemented communications.
> Though it seems that the fields deleted in this CL are not referenced
anywhere,
> including the server side code base, and are not mentioned in any plans or
> design docs for the future.
> So I'm fine with performing this removal.

Thanks, I guess it's fair game to take a look whether something is part of an
ongoing implementation, but if that's not the case I'd rather remove it (and
potentially add it back later).  Imho the same rules as for code should apply:
If it's unused, it either must get used (soon) or it must go away.

[Thiemo Nagel, 2017-04-21 09:48:34]

The CQ bit was checked by tnagel@chromium.org

[commit-bot: I haz the power, 2017-04-21 09:48:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-21 09:53:53]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1492768114687250,
"parent_rev": "1ddee16705605555f16c85012619ec1c14f3b582", "commit_rev":
"b92e7bbc8617e7b0a7c0e24f800457a51c420209"}

[commit-bot: I haz the power, 2017-04-21 09:54:03]

Description was changed from

==========
Remove obsolete .proto fields

BUG=713742
==========

to

==========
Remove obsolete .proto fields

BUG=713742

Review-Url: https://codereview.chromium.org/2828303002
Cr-Commit-Position: refs/heads/master@{#466299}
Committed:
https://chromium.googlesource.com/chromium/src/+/b92e7bbc8617e7b0a7c0e24f8004...
==========

[commit-bot: I haz the power, 2017-04-21 09:54:04]

Committed patchset #2 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/b92e7bbc8617e7b0a7c0e24f8004...