Review of "slinger" version 9

Details Page Preview

Sling windows around efficiently

Extension Homepage
https://github.com/timbertson/slinger

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
12 Active
11 Rejected
10 Rejected
9 Rejected
8 Rejected
7 Active
6 Active
5 Active
4 Active
3 Active
2 Active
1 Active

Previous Reviews on this Version

JustPerfection rejected
1. Please also include the xml file for schemas: [EGO Review Guidelines: schema](https://gjs.guide/extensions/review-guidelines/review-guidelines.html#gsettings-schemas) 2. Use `console.*` instead of `log()`: [Port Guide 45: Logging](https://gjs.guide/extensions/upgrading/gnome-shell-45.html#logging) 3. In some files you have unnecessary `;`: - line 13-20 `common.js` - line 21, 29 `extension_settings.js` 4. Don't use `var`. Use `let` and `const` instead. Use `export class` when you want to export class. 5. `lookupByURL()` is a bad practice (line 9 `extension_settings.js`). You can send `this` or `this.getSettings()` from the entry point to the class needing it when it's possible (dependency injection). 6. Timeout should be removed on destroy (line 159 `gnome_shell.js`): [EGO Review Guidelines: Timeout](https://gjs.guide/extensions/review-guidelines/review-guidelines.html#remove-main-loop-sources)
gfxmonk posted a review
I've addressed most of these in v11, except I don't understand #6. The timeout it for 10 milliseconds, it's not permanent and presumably someone's unlikely to disable the extension in that time?
JustPerfection posted a review
The timeout should be removed whether it's 10 milliseconds or the callback returns false since the timeout may get delayed and the callback can get triggered after disable (or in lock screen). As mentioned in the EGO Review Guidelines: > Note that all sources MUST be removed in disable(), even if the callback function will eventually return false or GLib.SOURCE_REMOVE.