Wrap TimeFormat unittests into unnamed namespace.

Wrap TimeFormat unittests into unnamed namespace.

Plus some other clean ups around.

R=phajdan.jr@chromium.org

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=106085

[Paweł Hajdan Jr., 2011-10-18 09:59:35]

http://codereview.chromium.org/8313003/diff/4/chrome/common/time_format.h
File chrome/common/time_format.h (right):

http://codereview.chromium.org/8313003/diff/4/chrome/common/time_format.h#new...
chrome/common/time_format.h:17: // This class defines methods to format time
values as strings.
nit: Omit "This class defines". Or maybe the whole thing should be changed from
a static class to a namespace?

http://codereview.chromium.org/8313003/diff/4/chrome/common/time_format_unitt...
File chrome/common/time_format_unittest.cc (right):

http://codereview.chromium.org/8313003/diff/4/chrome/common/time_format_unitt...
chrome/common/time_format_unittest.cc:5: #include "chrome/common/time_format.h"
nit: I don't think we put that header first in unit tests.

http://codereview.chromium.org/8313003/diff/4/chrome/common/time_format_unitt...
chrome/common/time_format_unittest.cc:51: TEST(TimeFormat, RelativeDate) {
nit: Why are you moving this?

[tfarina, 2011-10-18 16:00:53]

http://codereview.chromium.org/8313003/diff/4/chrome/common/time_format.h
File chrome/common/time_format.h (right):

http://codereview.chromium.org/8313003/diff/4/chrome/common/time_format.h#new...
chrome/common/time_format.h:17: // This class defines methods to format time
values as strings.
On 2011/10/18 09:59:35, Paweł Hajdan Jr. wrote:
> nit: Omit "This class defines". Or maybe the whole thing should be changed
from
> a static class to a namespace?
Yeah, that is what I had in mind for a follow up patch. But I was not sure if it
would be a good idea.

http://codereview.chromium.org/8313003/diff/4/chrome/common/time_format_unitt...
File chrome/common/time_format_unittest.cc (right):

http://codereview.chromium.org/8313003/diff/4/chrome/common/time_format_unitt...
chrome/common/time_format_unittest.cc:5: #include "chrome/common/time_format.h"
On 2011/10/18 09:59:35, Paweł Hajdan Jr. wrote:
> nit: I don't think we put that header first in unit tests.

Some says we should format like this (sky). Although, yes, the vast majority
don't do this way. But I have found it better, because it's easier to see which
API we're really testing.

http://codereview.chromium.org/8313003/diff/4/chrome/common/time_format_unitt...
chrome/common/time_format_unittest.cc:51: TEST(TimeFormat, RelativeDate) {
On 2011/10/18 09:59:35, Paweł Hajdan Jr. wrote:
> nit: Why are you moving this?
I think my idea was because of the order of the methods declared in the
TimeFormat API, which RelativeDate is the last one.

[Paweł Hajdan Jr., 2011-10-18 17:51:58]

LGTM