Issue 291553006: Don't augment GUID in ONC merging.

Issue 291553006: Don't augment GUID in ONC merging. (Closed)

Created:
6 years, 7 months ago by pneubeck (no reviews)

Modified:
6 years, 6 months ago

Reviewers:
stevenjb

CC:
chromium-reviews, chromium-apps-reviews_chromium.org, stevenjb+watch_chromium.org, oshima+watch_chromium.org, extensions-reviews_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

Don't augment GUID in ONC merging. The GUID property is used to pick user settings, policies and active settings for the same network. Thus it's more an identifier than a property and should not be exposed with additional meta information as it is the case for other properties. BUG=372337 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=274603

Patch Set 1 #

Total comments: 2

Patch Set 2 : Addressed comment. #

Created: 6 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+88 lines, -35 lines)			Patch
M	chrome/test/data/extensions/api_test/networking/test.js	View		1 chunk	+1 line, -5 lines	0 comments	Download
M	chromeos/network/onc/onc_merger.cc	View	1	5 chunks	+68 lines, -20 lines	0 comments	Download
M	chromeos/network/onc/onc_merger_unittest.cc	View		3 chunks	+3 lines, -1 line	0 comments	Download
M	chromeos/test/data/network/augmented_merge.json	View		5 chunks	+11 lines, -9 lines	0 comments	Download
A	chromeos/test/data/network/vpn_active_settings.onc	View		1 chunk	+5 lines, -0 lines	0 comments	Download

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

stevenjb

lgtm https://codereview.chromium.org/291553006/diff/1/chromeos/network/onc/onc_merger.cc File chromeos/network/onc/onc_merger.cc (right): https://codereview.chromium.org/291553006/diff/1/chromeos/network/onc/onc_merger.cc#newcode376 chromeos/network/onc/onc_merger.cc:376: effective_value->Equals(values.active_setting)); nit: Maybe wrap all of these checks ...

6 years, 7 months ago (2014-05-27 15:38:42 UTC) #2

pneubeck (no reviews)

The CQ bit was checked by pneubeck@chromium.org

6 years, 6 months ago (2014-06-03 16:21:25 UTC) #3

pneubeck (no reviews)

https://codereview.chromium.org/291553006/diff/1/chromeos/network/onc/onc_merger.cc File chromeos/network/onc/onc_merger.cc (right): https://codereview.chromium.org/291553006/diff/1/chromeos/network/onc/onc_merger.cc#newcode376 chromeos/network/onc/onc_merger.cc:376: effective_value->Equals(values.active_setting)); On 2014/05/27 15:38:43, stevenjb wrote: > nit: Maybe ...

6 years, 6 months ago (2014-06-03 16:21:29 UTC) #4

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/pneubeck@chromium.org/291553006/40001

6 years, 6 months ago (2014-06-03 16:21:36 UTC) #5

Message was sent while issue was closed.

Change committed as 274603

Expand Messages | Collapse Messages