Code review comment for lp:~albaguirre/unity8/add-screenshotter

Revision history for this message
Albert Astals Cid (aacid) wrote :

 * saveScreenshot
make QImage and QStrings there const &

 * ScreenGrabber::captureAndSave
Make QWindowList windows const so we get the "fast" operator[] when doing windows[0]

 * onKeyReleased:
bring the else up to the } line

 * screenGrabber.enable(Powerd.status === Powerd.On):
Looks like this could be a binding instead of a function call?

 * There's a few ; missing in the two onStopped functions

 * //TODO: This should be configurable (perhaps through gsettings?):
What's the use case/benefit for the user of making this configurable?

 * screenshotsDir.mkdir("Screenshots");
Should we localize this?

review: Needs Fixing

« Back to merge proposal