Review of "ShellTile" version 67

Details Page Preview

A tiling window extension for GNOME Shell. Just move a window to the edges of the screen to create a tiling, otherwise move a window over another one, holding down the Control key. Grouped windows minimize, resize, raise and change workspace together. Move or maximize a window to remove it from the group.

Extension Homepage
https://github.com/emasab/shelltile

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

Previous Reviews on this Version

emasab posted a review
1. What's the reason for line 679-681 extension.js? the main function was used in older versions o GNOME Shell that didn't have init, I've removed it. 2. You cannot create objects in the constructor of the class that `init()` returns (line 28-30, 42, 45 extension.js): https://gjs.guide/extensions/review-guidelines/review-guidelines.html#only-use-init-for-initialization Done 3. What's the reason for try catch blocks in enable and disable (line 266-312, 660-669 extension.js)? To log a possible exception, do I have to propagate the exception in that case? 4. You should remove the timeouts (also `idle_add`) in disable: https://gjs.guide/extensions/review-guidelines/review-guidelines.html#remove-main-loop-sources - line 158, 358, 384 and 575 extension.js - line 825, 959, tiling.js - line 157 window.js They're not called from disable and are needed when enabled because the events have to be processed after the rest of the callbacks in the event loop are handled. 5. Remove `package-lock.json` and `package.json`: https://gjs.guide/extensions/review-guidelines/review-guidelines.html#don-t-include-unnecessary-files Done 6. You cannot create objects in the global scope which is the same as init (line 54 util.js). Done 7. Don't create gsettings object in `init()` (prefs.js line 18). Make it local in `buildPrefsWidget` and pass it as dependency injection. Done 8. Multi versioning is supported here, so you can remove versions older than 3.36 if you want. That makes your job much easier for the future updates. I prefer to keep them as I'm trying to maintain compatibility with some older ones. 9. Lang is a deprecated module. Please remove it for the next version (supported on 3.32 and higher): https://gjs.guide/extensions/review-guidelines/review-guidelines.html#general-advice Learn how to remove Lang from your code: https://gjs.guide/guides/gjs/legacy-class-syntax.html Done 10. Use initTranslations() and getSettings() from ExtensionUtils instead of creating your own custom functions (supported on 3.32 and higher): https://gitlab.gnome.org/GNOME/gnome-shell/-/blob/main/js/misc/extensionUtils.js Done for versions that have ExtensionUtils
JustPerfection rejected
Thanks! 1. You should null out the objects you are creating in disable. For example: ```js this.gnome_shell_settings = null; this.gnome_mutter_settings = null; this.settings = null; ``` 2. For timeouts, you should remove them when the objects getting destroyed or extension is getting disabled. The timeouts can get delayed and the callback can get triggered after disable. That can lead to security issues or you can request access to a destroyed object.