Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(12)

Issue 2728463004: Remove enterprise serial number recovery feature (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 months, 3 weeks ago by pmarko
Modified:
4 weeks, 1 day ago
Reviewers:
Thiemo Nagel
CC:
chromium-reviews
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Remove enterprise serial number recovery feature Serial number recovery was introduced to recover the serial number of devices which did not provide the correct serial number on the initial enrollment request. This is not a problem anymore so the code can be removed. BUG=295504 TEST=Ran all unit_tests Review-Url: https://codereview.chromium.org/2728463004 Cr-Commit-Position: refs/heads/master@{#472929} Committed: https://chromium.googlesource.com/chromium/src/+/9911181ef43dfeaaabbd539f39fce567fc5d4e7b

Patch Set 1 #

Patch Set 2 : Remove enterprise serial number recovery feature #

Total comments: 4

Patch Set 3 : Rebase. #

Patch Set 4 : Addressed comments. #

Patch Set 5 : Removed access to removed fied in policy_testserver.py. #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -54 lines) Patch
M chrome/browser/chromeos/policy/device_local_account_policy_service_unittest.cc View 1 2 3 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/policy/test/policy_testserver.py View 1 2 3 4 4 chunks +0 lines, -20 lines 0 comments Download
M components/policy/core/common/cloud/cloud_policy_client.h View 2 chunks +0 lines, -7 lines 0 comments Download
M components/policy/core/common/cloud/cloud_policy_client.cc View 1 2 1 chunk +0 lines, -2 lines 0 comments Download
M components/policy/core/common/cloud/cloud_policy_client_unittest.cc View 1 2 1 chunk +0 lines, -2 lines 0 comments Download
M components/policy/core/common/cloud/cloud_policy_service.cc View 1 2 1 chunk +0 lines, -6 lines 0 comments Download
M components/policy/core/common/cloud/cloud_policy_service_unittest.cc View 1 2 1 chunk +0 lines, -2 lines 0 comments Download
M components/policy/core/common/cloud/mock_cloud_policy_client.h View 1 chunk +0 lines, -1 line 0 comments Download
M components/policy/proto/device_management_backend.proto View 1 2 3 3 chunks +2 lines, -12 lines 2 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 19 (6 generated)
pmarko
Hi Thiemo, please take a look at the changes when you have a few spare ...
3 months, 3 weeks ago (2017-03-02 17:40:09 UTC) #2
Thiemo Nagel
Sorry for the *very* slow review. (Btw, feel free to ping the reviewer when you've ...
1 month, 3 weeks ago (2017-05-02 10:53:07 UTC) #3
pmarko
No worries, this is not exactly an urgent change :-) PTAL. https://codereview.chromium.org/2728463004/diff/20001/components/policy/proto/device_management_backend.proto File components/policy/proto/device_management_backend.proto (right): ...
1 month ago (2017-05-17 16:00:21 UTC) #4
Thiemo Nagel
LGTM! Thank you very much!
1 month ago (2017-05-18 14:03:03 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2728463004/60001
1 month ago (2017-05-18 14:55:29 UTC) #7
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/448577)
1 month ago (2017-05-18 15:43:21 UTC) #9
pmarko
Thiemo, please take yet another look at the newest patchset - looks like I forgot ...
1 month ago (2017-05-18 16:38:41 UTC) #10
Thiemo Nagel
Still lgtm.
1 month ago (2017-05-18 16:54:39 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2728463004/80001
1 month ago (2017-05-18 16:57:58 UTC) #13
commit-bot: I haz the power
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/9911181ef43dfeaaabbd539f39fce567fc5d4e7b
1 month ago (2017-05-18 21:00:29 UTC) #16
Thiemo Nagel
https://codereview.chromium.org/2728463004/diff/80001/components/policy/proto/device_management_backend.proto File components/policy/proto/device_management_backend.proto (right): https://codereview.chromium.org/2728463004/diff/80001/components/policy/proto/device_management_backend.proto#newcode190 components/policy/proto/device_management_backend.proto:190: reserved 5, 10; Hey Pavol, I've just realized that ...
4 weeks, 1 day ago (2017-05-24 09:17:43 UTC) #17
pmarko
On 2017/05/24 09:17:43, Thiemo Nagel wrote: > https://codereview.chromium.org/2728463004/diff/80001/components/policy/proto/device_management_backend.proto > File components/policy/proto/device_management_backend.proto (right): > > https://codereview.chromium.org/2728463004/diff/80001/components/policy/proto/device_management_backend.proto#newcode190 ...
4 weeks, 1 day ago (2017-05-24 09:41:06 UTC) #18
pmarko
4 weeks, 1 day ago (2017-05-24 09:42:11 UTC) #19
Message was sent while issue was closed.
https://codereview.chromium.org/2728463004/diff/80001/components/policy/proto...
File components/policy/proto/device_management_backend.proto (right):

https://codereview.chromium.org/2728463004/diff/80001/components/policy/proto...
components/policy/proto/device_management_backend.proto:190: reserved 5, 10;
On 2017/05/24 09:17:42, Thiemo Nagel wrote:
> Hey Pavol, I've just realized that the "10" actually needs to go into
> PolicyData.  Could you please fix?

Good catch. Fix CL up:
https://chromium-review.googlesource.com/c/514082/
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cb946e318