Bug 59923

Comment on attachment 73717 [details] [review]
Save power

Review of attachment 73717 [details] [review]:
-----------------------------------------------------------------

Thanks for your attention to detail here, but we can't actually avoid more than one wakeup per run of Mission Control by making this change: this timeout only happens while shutting down MC. In practice, that only happens during the regression tests anyway.

If this is MC's only call to g_timeout_add() then you can mark the GNOME Goal as "done" already.

If you fix the comment and the constant naming as described below, I'd accept a patch anyway, but it isn't very important; or feel free to close this bug as WONTFIX if you don't think it's significant enough to bother about.

::: src/mcd-master.c
@@ +667,4 @@
>  }
>  
>  /* Milliseconds to wait for Connectivity coming back up before exiting MC */
> +#define EXIT_COUNTDOWN_TIME 5

The comment was already wrong (it's nothing to do with connectivity), but you've made it more wrong, by changing the constant to be a time in seconds while not amending the comment to match.

I'd also prefer a _SEC or _S suffix on the constant, to make it absolutely clear that it's in seconds (it should have been called ..._TIME_MS already, really).

It should be something like this:

/* Seconds to wait for everything to be freed during shutdown */
#define EXIT_COUNTDOWN_TIME_SEC 5

Not really a significant enough improvement to be worth leaving a bug open, but please reopen if you want to update the patch as suggested.