ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.
Mostly good, just a new nitpicks and suggestions:
INLINE COMMENTS
> powerdevilcore.cpp:256
> QString activity = m_activityConsumer->currentActivity();
> - qCDebug(POWERDEVIL) << "We are now into activity " << activity;
> + qCDebug(POWERDEVIL) << "We are now in activity " << activity; // into?
> direction?
> KConfigGroup activitiesConfig(m_profilesConfig, "Activities");
I'd say "using activity <whatever>"
> powerdevilcore.cpp:330
> + emitNotification(QStringLiteral("powerdevilerror"), i18n("Profile
> \"%1\" has been selected, "
> + "but the profile does not exist.\nPlease check your
> PowerDevil configuration.",
> profileId));
Remove "the profile"
> powerdevilcore.cpp:367
> << actionName << "a non-existent action.
> This is usually due to an installation problem,"
> - " or to a configuration problem, or simply
> the action is not supported";
> + " a configuration problem, or the action is
> not supported";
> }
"or the action is not supported" -> "or the action not being supported"
> powerdevilcore.cpp:541
> msg = i18nc("Placeholder is device name",
> - "The battery in your mouse (\"%1\") is low, and
> the device may turn itself off at any time. "
> - "Please replace or charge the battery as soon as
> possible.", name);
> + "The battery in (\"%1\") is running low, and the
> device may turn off at any time. "
> + "Please recharge or replace the battery.", name);
Are we 100% sure that `name` will always be a good enough string that the user
will recognize it as their mouse? (not a rhetorical question, I really am
wondering :) )
Same for other instances below too.
> powerdevilcore.cpp:607
> case PowerDevil::BundledActions::SuspendSession::ToDiskMode:
> - m_criticalBatteryNotification->setText(i18n("Your battery level is
> critical, the computer will be hibernated in 60 seconds."));
> + m_criticalBatteryNotification->setText(i18n("Battery level critical.
> Your computer will go into hibernation mode in 60 seconds."));
> m_criticalBatteryNotification->setActions(actions);
"go into" -> "enter" ?
> powerdevilcore.cpp:612
> case PowerDevil::BundledActions::SuspendSession::ToRamMode:
> - m_criticalBatteryNotification->setText(i18n("Your battery level is
> critical, the computer will be suspended in 60 seconds."));
> + m_criticalBatteryNotification->setText(i18n("Battery level critical.
> Your computer will go to sleep in 60 seconds.")); // go into sleep mode?
> m_criticalBatteryNotification->setActions(actions);
"go to sleep" is just fine here
> powerdevilcore.cpp:643
> } else {
> - emitRichNotification(QStringLiteral("pluggedin"), i18n("Running
> on AC power"), i18n("The power adaptor has been plugged in."));
> + emitRichNotification(QStringLiteral("pluggedin"), i18n("Running
> on AC power"), i18n("The power adapter has been plugged in.")); //uniform
> spelling throughout (either -er or -or, not both)
> }
Not sure this comment is really necessary :)
REPOSITORY
R122 Powerdevil
REVISION DETAIL
https://phabricator.kde.org/D19838
To: rooty, #vdg, #plasma, broulik, ngraham, filipf
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai,
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
