Review of "Grimble Tiling WM" version 2

Details Page Preview

You like GNOME? You like Tiling Window Managers? You can now have the best of both worlds with Grimble, the GNOME tiling window manager extension.

Extension Homepage
https://github.com/erwanlem/grimble-tiling-wm

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
5 Active
4 Rejected
3 Active
2 Rejected
1 Rejected

Previous Reviews on this Version

JustPerfection rejected
1. Please remove `schemas/gschemas.compiled`. Not needed. 2. Also null out in disable: ```js this._settings = null; this._switchHandler = null; this._spinHandler = null; ``` 3. You should clean up on disable or destroy: - line 45 `tileWindowManager.js` - line 4-5 `autocomplete.js` [EGO Review Guidelines: Destroy](https://gjs.guide/extensions/review-guidelines/review-guidelines.html#destroy-all-objects) 4. Putting `prefs/settings.js` into the prefs folder and loading it in GNOME Shell process is misleading for the reviewers. 5. `lookupByUUID()` is a bad practice. You are doing it in multiple files. You can send `this` from the entry point to the class needing it when it's possible (dependency injection). 6. Please remove unused properties (line 7-8 `prefs.js`). If you need any help with your extension you can ask us on: - [GNOME Extensions Matrix Channel](https://matrix.to/#/#extensions:gnome.org) - IRC Bridge: irc://irc.gimpnet.org/shell-extensions