Review of "Axe Menu" version 29

Details Page Preview

Big and beautiful menu for gnome-shell.

Extension Homepage
https://github.com/AxeMenu/

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

fmuellner active
Couple of notes (and I know most of those have already been present in the previous version): - compatibility checks are generally best done as feature checks rather than looking at a version number: TextDirection = (St.Widget.prototype.get_direction ? St.TextDirection.LTR : Clutter.TextDirection.LTR); getTextDirection = function(actor) { if (actor.get_direction) return actor.get_direction(); return actor.get_text_direction(); } etc. (Also note that all those checks are a bit pointless anyway, as they all apply to versions not listed in metadata.json anyway) - it's odd to keep compatibility stuff for old versions while ignoring compatibility with less older ones (St.IconType removal for instance); could do: if (this.icon.icon_type != undefined) this.icon.icon_type = St.IconType.SYMBOLIC; - you should use Main.wm.addKeybinding() instead of global.display.add_keybinding() when available (since 3.8) (and removeKeybinding() vs remove_keybinding() respectively) - testing for "GLib.get_home_dir() + '/.config'" and falling back to $HOME is ugly; the directory might not exist because the user explicitly configured a different directory, you should respect that. So IMHO it would be better to use something like: let configDir = GLib.get_user_config_dir(); if (GLib.mkdir_with_parents (configDir, 0700) < 0) configDir = GLib.get_home_dir(); let filename = configDir + '/axemenu.conf'; - why create your own configuration system in the first place instead of just using GSettings? - rather than implementing preferences as a modal dialog, you should seriously consider providing a prefs.js so users can access your preferences directly from extensions.gnome.org or the tweak tool as expected Anyway, none of those are a reason for rejecting the update, so approving.