Move ImportantFileWriter to base/. It now has two consumers, base/prefs and chrome/.

Move ImportantFileWriter to base/. It now has two consumers, base/prefs and chrome/.

TBR=ben@chromium.org
BUG=155525
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=163840

[brettw, 2012-10-23 05:22:47]

Is there any more stuff that will need to be moved or is this all the moving to
support prefs that will be required?

[Jói, 2012-10-23 10:58:13]

As far as I can tell, this is the last thing that will move to base/
and so the last one you will need to review.

There are a few more things that will move to base/prefs/ (where the
Prefs folks and myself are OWNERS) once their dependencies back on
Chrome have been broken; see the description of
http://codereview.chromium.org/11243002/ for a rough list.

Cheers,
Jói


On Tue, Oct 23, 2012 at 5:22 AM,  <brettw@chromium.org> wrote:
> Is there any more stuff that will need to be moved or is this all the moving
> to
> support prefs that will be required?
>
> http://codereview.chromium.org/11227026/

[brettw, 2012-10-23 17:16:32]

Okay, for the record I really hate this, but I don't have a better idea.

Long term, I'd like to move all the file stuff in base to base/files, so it
seems like that would be a good place to put this initially.

I'd also like to get this in the base namespace. Since there are relatively few
users, it seems like this may be pretty easy. I'm OK to do this in a followup.
We should also get the prefs service in the base namespace at some point.

[Jói, 2012-10-23 18:40:28]

I'll move ImportantFileWriter to the base namespace and to base/files
and send for review again when ready.

I have also put it on my list to move all the prefs stuff into the
base namespace, but this will probably happen towards the end of that
work.

Cheers,
Jói


On Tue, Oct 23, 2012 at 5:16 PM,  <brettw@chromium.org> wrote:
> Okay, for the record I really hate this, but I don't have a better idea.
>
> Long term, I'd like to move all the file stuff in base to base/files, so it
> seems like that would be a good place to put this initially.
>
> I'd also like to get this in the base namespace. Since there are relatively
> few
> users, it seems like this may be pretty easy. I'm OK to do this in a
> followup.
> We should also get the prefs service in the base namespace at some point.
>
> http://codereview.chromium.org/11227026/

[Jói, 2012-10-23 19:25:56]

PTAL, I have made the changes we discussed.

Cheers,
Jói

[brettw, 2012-10-24 03:30:53]

lgtm

http://codereview.chromium.org/11227026/diff/5004/base/files/important_file_w...
File base/files/important_file_writer.cc (right):

http://codereview.chromium.org/11227026/diff/5004/base/files/important_file_w...
base/files/important_file_writer.cc:89: namespace base {
I usually put this around the whole thing (including the anon namespace) so you
don't have to prefix stuff w/ "base::" in there.

[Jói, 2012-10-24 10:19:17]

http://codereview.chromium.org/11227026/diff/5004/base/files/important_file_w...
File base/files/important_file_writer.cc (right):

http://codereview.chromium.org/11227026/diff/5004/base/files/important_file_w...
base/files/important_file_writer.cc:89: namespace base {
On 2012/10/24 03:30:54, brettw wrote:
> I usually put this around the whole thing (including the anon namespace) so
you
> don't have to prefix stuff w/ "base::" in there.

I did that as well, in patch set 4, doing it differently was an attempt to fix a
really weird compile error on Windows.  It still pops up at line 154 instead,
see
http://build.chromium.org/p/tryserver.chromium/builders/win_rel/builds/73326/...

I'm putting the [ namespace base ] bit around the whole thing again, will figure
out the compiler confusion and then check in.

[Jói, 2012-10-24 13:46:54]

TBR=ben@ for mechanical updates (include paths, namespace).

[commit-bot: I haz the power, 2012-10-24 13:47:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joi@chromium.org/11227026/27001

[commit-bot: I haz the power, 2012-10-24 16:33:42]

Change committed as 163840

[Ben Goodger (Google), 2012-10-24 17:33:43]

lgtm