RegistryEntry: Made constructors public.

RegistryEntry: Made constructors public.

This makes it possible to use RegistryEntry to add new keys and values
to the Windows registry without having to add new static methods to
RegistryEntry itself.

The existing static methods will be moved out in a follow-up CL.

BUG=419972
Committed: https://crrev.com/e8703cdbca2eb16159cdad7135062f2389d2cfc0
Cr-Commit-Position: refs/heads/master@{#362124}

[Matt Giuca, 2015-11-30 02:42:07]

mgiuca@chromium.org changed reviewers:
+ gab@chromium.org

[Matt Giuca, 2015-11-30 02:42:07]

Next instalment in my re-do of https://codereview.chromium.org/623903002.

[Matt Giuca, 2015-11-30 02:54:15]

Description was changed from

==========
RegistryEntry: Made constructors public.

These are used internally by static methods, but those methods do not
really belong in RegistryEntry. As part of moving them out, I need to
make those constructors public.

BUG=419972
==========

to

==========
RegistryEntry: Made constructors public.

This makes it possible to use RegistryEntry to add new keys and values
to the Windows registry without having to add new static methods to
RegistryEntry itself.

The existing static methods will be moved out in a follow-up CL.

BUG=419972
==========

[gab, 2015-11-30 13:04:00]

lgtm, thanks

https://codereview.chromium.org/1486493002/diff/1/chrome/installer/util/shell...
File chrome/installer/util/shell_util.cc (right):

https://codereview.chromium.org/1486493002/diff/1/chrome/installer/util/shell...
chrome/installer/util/shell_util.cc:160: // directly, because if anything goes
wrong, they can be rolled back atomically.
I'd say remove the two "atomically" words in this comment.

It's not really "atomic", it's simply "tracked" and allows rollbacks which your
comment highlights without the word "atomically".

https://codereview.chromium.org/1486493002/diff/1/chrome/installer/util/shell...
chrome/installer/util/shell_util.cc:223: // Create a object that represent a key
of integer type
s/a object/an object/

here and above.

https://codereview.chromium.org/1486493002/diff/1/chrome/installer/util/shell...
chrome/installer/util/shell_util.cc:223: // Create a object that represent a key
of integer type
End comments with '.', here and above.

[gab, 2015-11-30 13:04:04]

The CQ bit was checked by gab@chromium.org

[commit-bot: I haz the power, 2015-11-30 13:04:21]

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

[commit-bot: I haz the power, 2015-11-30 13:45:24]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-11-30 13:46:13]

Description was changed from

==========
RegistryEntry: Made constructors public.

This makes it possible to use RegistryEntry to add new keys and values
to the Windows registry without having to add new static methods to
RegistryEntry itself.

The existing static methods will be moved out in a follow-up CL.

BUG=419972
==========

to

==========
RegistryEntry: Made constructors public.

This makes it possible to use RegistryEntry to add new keys and values
to the Windows registry without having to add new static methods to
RegistryEntry itself.

The existing static methods will be moved out in a follow-up CL.

BUG=419972

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

[commit-bot: I haz the power, 2015-11-30 13:46:14]

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

[Matt Giuca, 2015-12-01 00:46:35]

Umm did you LG with nits and then auto-land? Hard to fix the nits then ;)

I will send a quick follow-up CL.

https://codereview.chromium.org/1486493002/diff/1/chrome/installer/util/shell...
File chrome/installer/util/shell_util.cc (right):

https://codereview.chromium.org/1486493002/diff/1/chrome/installer/util/shell...
chrome/installer/util/shell_util.cc:160: // directly, because if anything goes
wrong, they can be rolled back atomically.
On 2015/11/30 13:04:00, gab wrote:
> I'd say remove the two "atomically" words in this comment.
> 
> It's not really "atomic", it's simply "tracked" and allows rollbacks which
your
> comment highlights without the word "atomically".

I think we want to highlight the transactional nature of this. You're right that
"atomically" isn't quite correct so replaced with "transactionally".

(Next CL.)

https://codereview.chromium.org/1486493002/diff/1/chrome/installer/util/shell...
chrome/installer/util/shell_util.cc:223: // Create a object that represent a key
of integer type
On 2015/11/30 13:04:00, gab wrote:
> End comments with '.', here and above.

Done (next CL).

Note: I wouldn't want to do this in the same CL as moving code. The whole reason
we got stuck last time is because I was doing too much in the one CL. Code moves
should just be moves and not make other changes, otherwise it is too hard to
review.

[gab, 2015-12-01 20:17:14]

On 2015/12/01 00:46:35, Matt Giuca wrote:
> Umm did you LG with nits and then auto-land? Hard to fix the nits then ;)

Oops, indeed, somehow forgot I'd dropped nits and am trying to CQ when it's a
clean l.g.t.m. to help you hit the ground with a fresh slate every morning on
the other side of the globe... A bit over eager on that one.. sorry!

> 
> I will send a quick follow-up CL.
> 
>
https://codereview.chromium.org/1486493002/diff/1/chrome/installer/util/shell...
> File chrome/installer/util/shell_util.cc (right):
> 
>
https://codereview.chromium.org/1486493002/diff/1/chrome/installer/util/shell...
> chrome/installer/util/shell_util.cc:160: // directly, because if anything goes
> wrong, they can be rolled back atomically.
> On 2015/11/30 13:04:00, gab wrote:
> > I'd say remove the two "atomically" words in this comment.
> > 
> > It's not really "atomic", it's simply "tracked" and allows rollbacks which
> your
> > comment highlights without the word "atomically".
> 
> I think we want to highlight the transactional nature of this. You're right
that
> "atomically" isn't quite correct so replaced with "transactionally".
> 
> (Next CL.)
> 
>
https://codereview.chromium.org/1486493002/diff/1/chrome/installer/util/shell...
> chrome/installer/util/shell_util.cc:223: // Create a object that represent a
key
> of integer type
> On 2015/11/30 13:04:00, gab wrote:
> > End comments with '.', here and above.
> 
> Done (next CL).
> 
> Note: I wouldn't want to do this in the same CL as moving code. The whole
reason
> we got stuck last time is because I was doing too much in the one CL. Code
moves
> should just be moves and not make other changes, otherwise it is too hard to
> review.

Agreed, as mentioned on the other CL: love the approach you're taking, but just
making sure we have the same end state in mind :-).

Thanks!
Gab

[Matt Giuca, 2015-12-01 22:56:13]

That's OK, I appreciate the auto-landing overnight.