Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[battery] Notify about charging completion only once. Fixes MER#1171 #363

Merged
merged 1 commit into from Jul 17, 2015

Conversation

deztructor
Copy link
Contributor

Do not notify on charging completion during maintenance charging cycles

Signed-off-by: Denis Zalevskiy denis.zalevskiy@jolla.com

@@ -153,6 +158,12 @@ void BatteryNotifier::prepareNotification()
checkChargingTimer->start();
}
}
if (isStateChanged) {
if (!isAnyChargingState)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In at least one case above, have used braces around one-line expression, so for consistency it may be best to use enclosing braces here too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chriadam I am using kernel conventions (no braces for 1-liners) to avoid extra syntax noise. But not a big deal to change it here, maybe it will be more consistent with the rest of the lipstick code.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big deal either way, whichever you prefer :-) Just thought I'd mention it.

Do not notify on charging completion during maintenance charging cycles

Signed-off-by: Denis Zalevskiy <denis.zalevskiy@jolla.com>
@chriadam
Copy link

LGTM although I'm not familiar with this code. It would be nice if someone who is could also take a look.

deztructor pushed a commit that referenced this pull request Jul 17, 2015
[battery] Notify about charging completion only once. Fixes MER#1171
@deztructor deztructor merged commit b638e46 into master Jul 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants