Cleanup formatting [chromiumos/platform/power_manager : master]

0 views
Skip to first unread message

Simon Que (Code Review)

unread,
May 14, 2012, 2:13:48 PM5/14/12
to
Simon Que has uploaded a new change for review.

Change subject: Cleanup formatting
......................................................................

Cleanup formatting

Made the following formatting changes:

- Pointer declarations should be "void* ptr" as opposed to "void *ptr".

- Statements that wrap to a new line should be indented by four spaces
unless otherwise aligned:
long_result_name =
LongExpressionThatWontFitOnOneLine(arg0, arg1, arg2, arg3);

- Fix mis-alignments.

- Reduced number of lines with more efficient expression wrapping.

- Removed unnecessary parentheses around return values.

- Put parentheses around comparison expressions that are used as
rvalues:
bool result = (actual_value == expected_value);

- Declare variables as they are used, rather than at the top of a
function.

- Avoid abbreviations like "ev", "retval", and "cmd". Acronyms like
"fd" are retained.

- Remove unsed parameter names in function declarations.

BUG=chromium-os:28305
TEST=build, unit tests pass.

Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Signed-off-by: Simon Que <sq...@chromium.org>
---
M backlight_dbus_tool.cc
M external_backlight.cc
M file_tagger.cc
M file_tagger.h
M inotify.cc
M inotify.h
M input.cc
M internal_backlight_controller.cc
M mock_xsync.cc
M mock_xsync.h
M monitor_reconfigure.cc
M power_supply.cc
M power_supply.h
M powerd.cc
M powerd.h
M powerman.cc
M state_control.cc
M util.cc
M util.h
M util_dbus.cc
M xidle.cc
M xsync.cc
M xsync.h
M xsync_interface.h
24 files changed, 115 insertions(+), 121 deletions(-)


git pull ssh://gerrit.chromium.org:29418/chromiumos/platform/power_manager refs/changes/95/22595/1
--
To view, visit https://gerrit.chromium.org/gerrit/22595
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/platform/power_manager
Gerrit-Branch: master
Gerrit-Owner: Simon Que <sq...@chromium.org>

Thiago Farina (Code Review)

unread,
May 14, 2012, 2:15:53 PM5/14/12
to Simon Que, Thiago Farina
Thiago Farina has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 1:

"Remove unsed parameter names in function declarations." I think cpplint complains about this.

--
To view, visit https://gerrit.chromium.org/gerrit/22595
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/platform/power_manager
Gerrit-Branch: master
Gerrit-Owner: Simon Que <sq...@chromium.org>
Gerrit-Reviewer: Thiago Farina <tfa...@chromium.org>

Thiago Farina (Code Review)

unread,
May 14, 2012, 2:18:05 PM5/14/12
to Simon Que, Jon Kliegman, Thiago Farina, Ryan Harrison
Thiago Farina has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 1: (2 inline comments)

....................................................
File file_tagger.h
Line 54: static gboolean TraceFileChangeHandler(const char*, int, unsigned int,
nit: I don't think removing the parameter name is a good idea :(
and one arg per line?

....................................................
File powerd.h
Line 315: static gboolean PrefChangeHandler(const char* name, int, unsigned int,
nit: one arg per line?

--
To view, visit https://gerrit.chromium.org/gerrit/22595
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/platform/power_manager
Gerrit-Branch: master
Gerrit-Owner: Simon Que <sq...@chromium.org>
Gerrit-Reviewer: Jon Kliegman <kli...@chromium.org>
Gerrit-Reviewer: Ryan Harrison <rhar...@chromium.org>
Gerrit-Reviewer: Thiago Farina <tfa...@chromium.org>

Jon Kliegman (Code Review)

unread,
May 14, 2012, 2:30:21 PM5/14/12
to Simon Que, Thiago Farina, Ryan Harrison
Jon Kliegman has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 1: (2 inline comments)

About hallfway through but figured I'd commit before I headed to a meeting. Will continue afterwards.

....................................................
File backlight_dbus_tool.cc
Line 106: double percent;
I couldn't fix an exact description in the style guide, but since this is the same scope as where 'int style' is defined i'd like to see it stay there

....................................................
File file_tagger.h
Line 54: static gboolean TraceFileChangeHandler(const char*, int, unsigned int,
If the names aren't used they should be included in comments.

So TraceFileChangeHandler(const char* /* name */, int /* wd */, ...)

As for one arg per line I'm pretty sure that's just optional and I think we mentioned this elsewhere. If its becoming a convention in chromium code then the style guide should be updated. Otherwise stick with the existing convention in files which allow multiple arguments per line.

Related to this, arguments & types are usually much shorter in this repo than in Chromium (no renderhostviewswidgethostviewsview or whatever) so having multiple args/line wouldn't impact readability as much.

--
To view, visit https://gerrit.chromium.org/gerrit/22595
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/platform/power_manager
Gerrit-Branch: master
Gerrit-Owner: Simon Que <sq...@chromium.org>

Jon Kliegman (Code Review)

unread,
May 14, 2012, 5:22:31 PM5/14/12
to Simon Que, Thiago Farina, Ryan Harrison
Jon Kliegman has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 1: (4 inline comments)

....................................................
File file_tagger.cc
Line 87: unsigned int, // mask
The style guide shows it as:
...
int /*watch_handle*/,
unsigned int /*mask*/,
...

....................................................
File powerman.cc
Line 567: char suspend_command[60];
This should probably remain at the top where we enter this scope

....................................................
File util_dbus.cc
Line 43: result = (std::string(state) == "started");
Any reason we can't use strcmp (or strncmp) here? constructing a string object just to compare it to a string literal seems inefficient.

Line 174: return true;
Sorry about these :( Bad habit.

--
To view, visit https://gerrit.chromium.org/gerrit/22595
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/platform/power_manager
Gerrit-Branch: master
Gerrit-Owner: Simon Que <sq...@chromium.org>

Ryan Harrison (Code Review)

unread,
May 15, 2012, 10:26:04 AM5/15/12
to Simon Que, Jon Kliegman, Thiago Farina
Ryan Harrison has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 1: (2 inline comments)

....................................................
File backlight_dbus_tool.cc
Line 106: double percent;
I agree that there isn't a clear guide ruling on this, but I think where double percent has been moved to is more appropriate, since it isn't used until this point. This I think is more in keeping with what I have seen in this and other packages, though I don't doubt there is counter-examples out there.

....................................................
File powerman.cc
Line 567: char suspend_command[60];
Again, I think the convention has been to declare variables local to their first usage.

--
To view, visit https://gerrit.chromium.org/gerrit/22595
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/platform/power_manager
Gerrit-Branch: master
Gerrit-Owner: Simon Que <sq...@chromium.org>

Jon Kliegman (Code Review)

unread,
May 15, 2012, 11:16:49 AM5/15/12
to Simon Que, Thiago Farina, Ryan Harrison
Jon Kliegman has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 1:

Conceding the point to ryan on the two local variable declaration positions. Style guide just says declare at same scope and I generally interpret that as "declare at start of scope its used in" but it can go either way and should follow the style of the rest of the file. Feel free to leave as is.

--
To view, visit https://gerrit.chromium.org/gerrit/22595
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/platform/power_manager
Gerrit-Branch: master
Gerrit-Owner: Simon Que <sq...@chromium.org>

Simon Que (Code Review)

unread,
May 15, 2012, 4:18:10 PM5/15/12
to Jon Kliegman, Thiago Farina, Ryan Harrison
Simon Que has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 1: (2 inline comments)

....................................................
File backlight_dbus_tool.cc
Line 106: double percent;
This is what I found:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Local_Variables#Local_Variables

"initialization should be used instead of declaration and assignment"
We could have:

int style = (FLAGS_gradual) ? power_manager::kBrightnessTransitionGradual
: power_manager::kBrightnessTransitionInstant;

....................................................
File powerman.cc
Line 567: char suspend_command[60];
"We encourage you to declare them in as local a scope as possible, and as close to the first use as possible."
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Local_Variables

--
To view, visit https://gerrit.chromium.org/gerrit/22595
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/platform/power_manager
Gerrit-Branch: master
Gerrit-Owner: Simon Que <sq...@chromium.org>
Gerrit-Reviewer: Jon Kliegman <kli...@chromium.org>
Gerrit-Reviewer: Ryan Harrison <rhar...@chromium.org>
Gerrit-Reviewer: Simon Que <sq...@chromium.org>
Gerrit-Reviewer: Thiago Farina <tfa...@chromium.org>

Jon Kliegman (Code Review)

unread,
May 15, 2012, 4:27:51 PM5/15/12
to Simon Que, Thiago Farina, Ryan Harrison
Jon Kliegman has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 1: (2 inline comments)

....................................................
File backlight_dbus_tool.cc
Line 106: double percent;
Does FLAGS_gradual need parens here? Otherwise lgtm

....................................................
File powerman.cc
Line 567: char suspend_command[60];
I stand corrected and ashamed.

--
To view, visit https://gerrit.chromium.org/gerrit/22595
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/platform/power_manager
Gerrit-Branch: master
Gerrit-Owner: Simon Que <sq...@chromium.org>

Simon Que (Code Review)

unread,
May 15, 2012, 4:49:01 PM5/15/12
to Jon Kliegman, Thiago Farina, Ryan Harrison
Simon Que has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 1: (4 inline comments)

....................................................
File backlight_dbus_tool.cc
Line 106: double percent;
Done

....................................................
File file_tagger.cc
Line 87: unsigned int, // mask
Done

....................................................
File file_tagger.h
Line 54: static gboolean TraceFileChangeHandler(const char*, int, unsigned int,
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Function_Declarations_and_Definitions

"If some parameters are unused, comment out the variable name in the function definition"

However the example shows the variable name being retained in the declaration.

....................................................
File util_dbus.cc
Line 43: result = (std::string(state) == "started");
Done

--
To view, visit https://gerrit.chromium.org/gerrit/22595
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/platform/power_manager
Gerrit-Branch: master
Gerrit-Owner: Simon Que <sq...@chromium.org>

Ryan Harrison (Code Review)

unread,
May 16, 2012, 10:50:46 AM5/16/12
to Simon Que, Jon Kliegman, Thiago Farina, Benson Leung
Ryan Harrison has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 3: Looks good to me, but someone else must approve

--
To view, visit https://gerrit.chromium.org/gerrit/22595
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Gerrit-PatchSet: 3
Gerrit-Project: chromiumos/platform/power_manager
Gerrit-Branch: master
Gerrit-Owner: Simon Que <sq...@chromium.org>
Gerrit-Reviewer: Benson Leung <ble...@chromium.org>

Jon Kliegman (Code Review)

unread,
May 16, 2012, 10:51:18 AM5/16/12
to Simon Que, Thiago Farina, Ryan Harrison, Benson Leung
Jon Kliegman has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 3: Looks good to me, approved

(1 inline comment)

....................................................
File file_tagger.cc
Line 86: int /* wd (watch handle) */,
NIT: Change the other locations to use watch_handle instead of wd to avoid the parens?

--
To view, visit https://gerrit.chromium.org/gerrit/22595
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Gerrit-PatchSet: 3
Gerrit-Project: chromiumos/platform/power_manager
Gerrit-Branch: master
Gerrit-Owner: Simon Que <sq...@chromium.org>

Simon Que (Code Review)

unread,
May 16, 2012, 3:37:48 PM5/16/12
to Jon Kliegman, Thiago Farina, Ryan Harrison, Benson Leung
Simon Que has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 3: (1 inline comment)

....................................................
File file_tagger.cc
Line 86: int /* wd (watch handle) */,
Done

--
To view, visit https://gerrit.chromium.org/gerrit/22595
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Gerrit-PatchSet: 3
Gerrit-Project: chromiumos/platform/power_manager
Gerrit-Branch: master
Gerrit-Owner: Simon Que <sq...@chromium.org>

Jon Kliegman (Code Review)

unread,
May 16, 2012, 3:39:22 PM5/16/12
to Simon Que, Thiago Farina, Ryan Harrison, Benson Leung
Jon Kliegman has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 4: Looks good to me, approved

--
To view, visit https://gerrit.chromium.org/gerrit/22595
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Gerrit-PatchSet: 4
Gerrit-Project: chromiumos/platform/power_manager
Gerrit-Branch: master
Gerrit-Owner: Simon Que <sq...@chromium.org>

Jon Kliegman (Code Review)

unread,
May 16, 2012, 3:39:40 PM5/16/12
to Simon Que, Thiago Farina, Ryan Harrison, Benson Leung
Jon Kliegman has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 4:

Thanks for the cleanup - much appreciated.

--
To view, visit https://gerrit.chromium.org/gerrit/22595
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Gerrit-PatchSet: 4
Gerrit-Project: chromiumos/platform/power_manager
Gerrit-Branch: master
Gerrit-Owner: Simon Que <sq...@chromium.org>

Ryan Harrison (Code Review)

unread,
May 16, 2012, 3:42:50 PM5/16/12
to Simon Que, Jon Kliegman, Thiago Farina, Benson Leung
Ryan Harrison has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 4: Looks good to me, but someone else must approve

--
To view, visit https://gerrit.chromium.org/gerrit/22595
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Gerrit-PatchSet: 4
Gerrit-Project: chromiumos/platform/power_manager
Gerrit-Branch: master
Gerrit-Owner: Simon Que <sq...@chromium.org>

Simon Que (Code Review)

unread,
May 16, 2012, 4:55:45 PM5/16/12
to Jon Kliegman, Thiago Farina, Ryan Harrison, Benson Leung
Simon Que has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 4: Verified; Ready

--
To view, visit https://gerrit.chromium.org/gerrit/22595
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Gerrit-PatchSet: 4
Gerrit-Project: chromiumos/platform/power_manager
Gerrit-Branch: master
Gerrit-Owner: Simon Que <sq...@chromium.org>

Benson Leung (Code Review)

unread,
May 16, 2012, 5:08:26 PM5/16/12
to Simon Que, Jon Kliegman, Thiago Farina, Ryan Harrison
Benson Leung has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 4: Looks good to me, approved

--
To view, visit https://gerrit.chromium.org/gerrit/22595
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Gerrit-PatchSet: 4
Gerrit-Project: chromiumos/platform/power_manager
Gerrit-Branch: master
Gerrit-Owner: Simon Que <sq...@chromium.org>

Simon Que (Code Review)

unread,
May 16, 2012, 5:44:32 PM5/16/12
to Jon Kliegman, Thiago Farina, Ryan Harrison, Benson Leung
Simon Que has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 4: (3 inline comments)

....................................................
File input.cc
Line 148: bool result = true;
I removed result since it's used in fewer places.

Line 172: result = false;
This becomes "return false;"

Line 176: return result;
This becomes "return true;"

--
To view, visit https://gerrit.chromium.org/gerrit/22595
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Gerrit-PatchSet: 4
Gerrit-Project: chromiumos/platform/power_manager
Gerrit-Branch: master
Gerrit-Owner: Simon Que <sq...@chromium.org>

Simon Que (Code Review)

unread,
May 16, 2012, 5:46:46 PM5/16/12
to Jon Kliegman, Thiago Farina, Ryan Harrison, Benson Leung
Simon Que has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 5: Verified; Looks good to me, approved; Ready

Rebased

--
To view, visit https://gerrit.chromium.org/gerrit/22595
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Gerrit-PatchSet: 5
Gerrit-Project: chromiumos/platform/power_manager
Gerrit-Branch: master
Gerrit-Owner: Simon Que <sq...@chromium.org>

Jon Kliegman (Code Review)

unread,
May 16, 2012, 5:49:20 PM5/16/12
to Simon Que, Thiago Farina, Ryan Harrison, Benson Leung
Jon Kliegman has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 5: (1 inline comment)

....................................................
File input.cc
Line 167: LOG(ERROR) << "No lid events registered.";
This error looks odd. num_lid_events_ >1 but saying none? Although it seems unrelated to your change

--
To view, visit https://gerrit.chromium.org/gerrit/22595
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Gerrit-PatchSet: 5
Gerrit-Project: chromiumos/platform/power_manager
Gerrit-Branch: master
Gerrit-Owner: Simon Que <sq...@chromium.org>

Jon Kliegman (Code Review)

unread,
May 16, 2012, 5:49:41 PM5/16/12
to Simon Que, Thiago Farina, Ryan Harrison, Benson Leung
Jon Kliegman has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 5: Looks good to me, approved

--
To view, visit https://gerrit.chromium.org/gerrit/22595
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Gerrit-PatchSet: 5
Gerrit-Project: chromiumos/platform/power_manager
Gerrit-Branch: master
Gerrit-Owner: Simon Que <sq...@chromium.org>

Mike Frysinger (Code Review)

unread,
May 16, 2012, 6:21:22 PM5/16/12
to Simon Que, Jon Kliegman, Thiago Farina, Ryan Harrison, Benson Leung, Mike Frysinger
Mike Frysinger has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 5: Fails

breaks the CQ:

power_manager-0.0.1-r400: ../input.cc: In static member function 'static gboolean power_manager::Input::EventHandler(GIOChannel*, GIOCondition, gpointer)':
power_manager-0.0.1-r400: ../input.cc:514:60: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]

--
To view, visit https://gerrit.chromium.org/gerrit/22595
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Gerrit-PatchSet: 5
Gerrit-Project: chromiumos/platform/power_manager
Gerrit-Branch: master
Gerrit-Owner: Simon Que <sq...@chromium.org>
Gerrit-Reviewer: Benson Leung <ble...@chromium.org>
Gerrit-Reviewer: Jon Kliegman <kli...@chromium.org>
Gerrit-Reviewer: Mike Frysinger <vap...@chromium.org>

Simon Que (Code Review)

unread,
May 16, 2012, 6:22:55 PM5/16/12
to Jon Kliegman, Thiago Farina, Ryan Harrison, Benson Leung, Mike Frysinger
Simon Que has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 5:

Strange that it fails. The CL doesn't change anything functionally in that part of the code.

--
To view, visit https://gerrit.chromium.org/gerrit/22595
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Gerrit-PatchSet: 5
Gerrit-Project: chromiumos/platform/power_manager
Gerrit-Branch: master
Gerrit-Owner: Simon Que <sq...@chromium.org>

Jon Kliegman (Code Review)

unread,
May 16, 2012, 6:43:15 PM5/16/12
to Simon Que, Thiago Farina, Ryan Harrison, Benson Leung
Jon Kliegman has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 6: Looks good to me, approved

I'm a bit worried if it built locally for you but not on the bot - we should be building with the same flags.

Also don't forget trybot runs! I forgot my normal copy paste of them.

--
To view, visit https://gerrit.chromium.org/gerrit/22595
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Gerrit-PatchSet: 6
Gerrit-Project: chromiumos/platform/power_manager
Gerrit-Branch: master
Gerrit-Owner: Simon Que <sq...@chromium.org>
Gerrit-Reviewer: Benson Leung <ble...@chromium.org>
Gerrit-Reviewer: Jon Kliegman <kli...@chromium.org>

Grant Grundler (Code Review)

unread,
May 16, 2012, 6:58:59 PM5/16/12
to Simon Que, Jon Kliegman, Thiago Farina, Ryan Harrison, Benson Leung, Grant Grundler
Grant Grundler has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 6:

bots failed to build. Benson suspects this CL might be the cause. Here's the log for the build failure:
http://chromegw/i/chromiumos/builders/x86%20generic%20paladin/builds/4912/steps/BuildTarget/logs/stdio

--
To view, visit https://gerrit.chromium.org/gerrit/22595
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I060fbdd6b40c56bcafeee9fd60fd00a0cd155e7a
Gerrit-PatchSet: 6
Gerrit-Project: chromiumos/platform/power_manager
Gerrit-Branch: master
Gerrit-Owner: Simon Que <sq...@chromium.org>
Gerrit-Reviewer: Benson Leung <ble...@chromium.org>
Gerrit-Reviewer: Grant Grundler <grun...@chromium.org>

Grant Grundler (Code Review)

unread,
May 16, 2012, 7:00:02 PM5/16/12
to Simon Que, Jon Kliegman, Thiago Farina, Ryan Harrison, Benson Leung, Grant Grundler
Grant Grundler has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 6:

Sorry...should have included the error message:
power_manager-0.0.1-r400: ../input.cc: In static member function 'static gboolean power_manager::Input::EventHandler(GIOChannel*, GIOCondition, gpointer)':
power_manager-0.0.1-r400: ../input.cc:514:60: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
power_manager-0.0.1-r400: cc1plus: all warnings being treated as errors

Simon Que (Code Review)

unread,
May 16, 2012, 7:09:45 PM5/16/12
to Jon Kliegman, Thiago Farina, Ryan Harrison, Benson Leung, Grant Grundler
Simon Que has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 6: Verified; Ready

Patch set 6 passed amd64-generic, x86-generic, lumpy, and alex paladins

Simon Que (Code Review)

unread,
May 16, 2012, 8:07:25 PM5/16/12
to Jon Kliegman, Thiago Farina, Ryan Harrison, Benson Leung, Grant Grundler
Simon Que has posted comments on this change.

Change subject: Cleanup formatting
......................................................................


Patch Set 6: Ready
Reply all
Reply to author
Forward
0 new messages