[Sync] Remove ScopedServerStatusWatcher

[Sync] Remove ScopedServerStatusWatcher

Also deprecate a few error types that are not returned from server.

Without this class it is more clear how server_status gets from
HttpResponse to ServerConnectionManager's server_status.

R=zea@chromium.org,timonvo@chromium.org
BUG=567273
Committed: https://crrev.com/ea4e35e99416f2dc84bda02d9b9a4b5e617317cd
Cr-Commit-Position: refs/heads/master@{#363713}

[pavely, 2015-12-07 22:22:27]

Nicolas, PTAL.
Timon, I've included you for sync_enums.proto changes. I deprecated few
ErrorType constants.

[timonvo, 2015-12-07 22:26:38]

On 2015/12/07 22:22:27, pavely wrote:
> Nicolas, PTAL.
> Timon, I've included you for sync_enums.proto changes. I deprecated few
> ErrorType constants.

LGTM for sync/protocol/sync_enums.proto changes.

[Nicolas Zea, 2015-12-07 23:10:19]

LGTM with a proto suggestion

https://codereview.chromium.org/1505953002/diff/1/sync/protocol/sync_enums.proto
File sync/protocol/sync_enums.proto (right):

https://codereview.chromium.org/1505953002/diff/1/sync/protocol/sync_enums.pr...
sync/protocol/sync_enums.proto:80: DEPRECATED_1          = 1;  // Was
ACCESS_DENIED.
I haven't seen this naming pattern before for deprecating protos. Did you get it
from somewhere in particular? Typically we either simply comment the field out
altogether, or keep the name but at the DEPTRECATED_ prefix.

[timonvo, 2015-12-07 23:15:17]

https://codereview.chromium.org/1505953002/diff/1/sync/protocol/sync_enums.proto
File sync/protocol/sync_enums.proto (right):

https://codereview.chromium.org/1505953002/diff/1/sync/protocol/sync_enums.pr...
sync/protocol/sync_enums.proto:80: DEPRECATED_1          = 1;  // Was
ACCESS_DENIED.
On 2015/12/07 23:10:19, Nicolas Zea wrote:
> I haven't seen this naming pattern before for deprecating protos. Did you get
it
> from somewhere in particular? Typically we either simply comment the field out
> altogether, or keep the name but at the DEPTRECATED_ prefix.

FWIW, we've used this pattern elsewhere on the server (and I think in some of
our protos here). I like it, since it removes any confusion and makes it clear
these values should not be used at all anymore.

Now you did make me think though, we should also add "[deprecated = true]" after
the tag number to be 100% complete.

[pavely, 2015-12-08 00:19:06]

Patchset #2 (id:20001) has been deleted

[pavely, 2015-12-08 01:04:26]

On 2015/12/07 23:15:17, timonvo wrote:
>
https://codereview.chromium.org/1505953002/diff/1/sync/protocol/sync_enums.proto
> File sync/protocol/sync_enums.proto (right):
> 
>
https://codereview.chromium.org/1505953002/diff/1/sync/protocol/sync_enums.pr...
> sync/protocol/sync_enums.proto:80: DEPRECATED_1          = 1;  // Was
> ACCESS_DENIED.
> On 2015/12/07 23:10:19, Nicolas Zea wrote:
> > I haven't seen this naming pattern before for deprecating protos. Did you
get
> it
> > from somewhere in particular? Typically we either simply comment the field
out
> > altogether, or keep the name but at the DEPTRECATED_ prefix.
> 
> FWIW, we've used this pattern elsewhere on the server (and I think in some of
> our protos here). I like it, since it removes any confusion and makes it clear
> these values should not be used at all anymore.
> 
> Now you did make me think though, we should also add "[deprecated = true]"
after
> the tag number to be 100% complete.

I checked, DEPRECATED_name format is used much more often than DEPRECATED_N so I
changed it.
[deprecated =true] option can be applied to field but not to enum value (at
least on client).

[pavely, 2015-12-08 02:06:11]

The CQ bit was checked by pavely@chromium.org

[pavely, 2015-12-08 02:06:11]

The patchset sent to the CQ was uploaded after l-g-t-m from
timonvo@chromium.org, zea@chromium.org
Link to the patchset: https://codereview.chromium.org/1505953002/#ps40001
(title: "Change format of deprecated names.")

[commit-bot: I haz the power, 2015-12-08 02:08:31]

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

[commit-bot: I haz the power, 2015-12-08 02:35:57]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2015-12-08 02:36:49]

Description was changed from

==========
[Sync] Remove ScopedServerStatusWatcher

Also deprecate a few error types that are not returned from server.

Without this class it is more clear how server_status gets from
HttpResponse to ServerConnectionManager's server_status.

R=zea@chromium.org,timonvo@chromium.org
BUG=567273
==========

to

==========
[Sync] Remove ScopedServerStatusWatcher

Also deprecate a few error types that are not returned from server.

Without this class it is more clear how server_status gets from
HttpResponse to ServerConnectionManager's server_status.

R=zea@chromium.org,timonvo@chromium.org
BUG=567273

Committed: https://crrev.com/ea4e35e99416f2dc84bda02d9b9a4b5e617317cd
Cr-Commit-Position: refs/heads/master@{#363713}
==========

[commit-bot: I haz the power, 2015-12-08 02:36:50]

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