Review of "Espresso" version 5

Details Page Preview

Espresso disables the usual auto suspend and screensaver functionality and optionally Night Light with options to show an Espresso icon in the top panel, to enable Espresso when a fullscreen application is running, to restore state across reboots, to provide notifications, to enable Espresso when specific applications are running, or to pause Night Light when Espresso is enabled or only when specific applications are running. Espresso also provides some support for docking stations including options to enable Espresso when charging and/or when docked to external monitors and to allow temporarily overriding the docking support without affecting the stored state. Espresso is a fork of the Caffeine extension. Please leave feedback or report issues through the Extension Homepage

Extension Homepage
https://github.com/coadmunkee/gnome-shell-extension-espresso

No comments.

Diff Against

Files

Note: Binary files aren't shown on the web site. To see all files, please download the extension zipfile.

All Versions

Version Status
10 Active
9 Active
8 Active
7 Active
6 Inactive
5 Rejected
4 Inactive
3 Rejected
2 Rejected
1 Inactive

Previous Reviews on this Version

JustPerfection rejected
1. Timeouts in line 198, 310 and 339 (extension.js) should be removed on disable: https://gjs.guide/extensions/review-guidelines/review-guidelines.html#remove-main-loop-sources 2. It's better to do line 79-85 (extension.js) in enable and null them out in disable. or you can use dependency injection. 3. Use a helper function for log instead of using `if (ENABLE_DEBUG)` every time. If you need any help with your extension you can ask us on: - [GNOME Matrix Channel](https://matrix.to/#/#extensions:gnome.org) - IRC Bridge: irc://irc.gimpnet.org/shell-extensions
espresso posted a review
I have updated my development copy to address points 1 and 3. However I am having some trouble trying to accommodate item 2. Moving the makeProxyWrapper calls on lines 79-85 into enable causes issues becasue the reusable classes created by these calls are not available within _init Is it acceptable to lines 79-85 as they are, or do I need to go get on IRC and look for some help to come up with a better solution before this could be approved?
JustPerfection posted a review
Why not creating them in enable and sending them as dependency injection? ```js EspressoIndicator = new Espresso(DBusSessionManagerProxy, PowerManagerProxy, DBusSessionManagerInhibitorProxy); ```
espresso posted a review
Can you explain to me why it is better to create these class definitions in enable? I honestly don't understand why it is 'better' to do it that way. I have spent two full days trying to make the approach work and every method I've tried for moving the proxy class definition to enable and passing through dependency injection results in session managers that don't function. Instantiating the proxy wrapper classes like this is not entirely new with this version of the extension (the PowerManager proxy wrapper is new, but the other two are preexisting). It's also the same approach the Caffeine extension uses for it's current version and many prior versions. I have made more than an honest effort at making this work and haven't been successful.
JustPerfection posted a review
Send it with #1 and #3 fix.