Review of "AppMenu" version 2

Details Page Preview

A zero-dependency macOS-style global menu bar for GNOME Shell with app-aware menus, shortcuts, recent items, workspaces, and fast user switching.

Extension Homepage
https://github.com/ChathurangaBW/AppMenu

No comments.

Diff Against

Files

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

Shexli (experimental) warning 6

Shexli found 6 issues that may need reviewer attention.

EGO-X-004 warning

extensions should avoid synchronous file IO in shell code

Shell code should avoid synchronous file IO APIs like `GLib.file_get_contents()` and `Gio.File.load_contents()`.

File Operations

  • menuManager.js:53
    GLib.file_get_contents('/etc/os-release')
  • searchDialog.js:39
    GLib.file_get_contents(RECENT_ITEMS_FILE)

EGO-X-003 warning

extensions should not call run_dispose in extension code

Extension code should not call `run_dispose()`.

GObject.Object.run_dispose

  • realMenuManager.js:705
    this._client.run_dispose()

EGO-P-006 warning

unnecessary build and translation artifacts should not be shipped

Compiled GSettings schemas should not be shipped for 45+ packages.

Don't include unnecessary files

  • schemas/gschemas.compiled
    schemas/gschemas.compiled

EGO-L-002 warning

objects created by extension should be destroyed in disable()

Objects assigned in `enable()` are missing matching `.destroy()` calls in `disable()` or its helper methods.

Destroy all objects

  • searchDialog.js:64
            this._entry = new St.Entry({
                style_class: 'appmenu-search-entry',
                hint_text: 'Search apps, recent files, and settings',
                can_focus: true,
                track_hover: true,
            })
  • searchDialog.js:72
            this._resultsBox = new St.BoxLayout({
                vertical: true,
                style_class: 'appmenu-search-results',
            })
  • searchDialog.js:275
    button

EGO-L-005 warning

owned object references should be released in disable()

Owned references that are cleaned up in `disable()` should also be released with `null` or `undefined`.

Destroy all objects

  • searchDialog.js:59
    this._appSystem = Shell.AppSystem.get_default()
  • searchDialog.js:64
            this._entry = new St.Entry({
                style_class: 'appmenu-search-entry',
                hint_text: 'Search apps, recent files, and settings',
                can_focus: true,
                track_hover: true,
            })
  • searchDialog.js:72
            this._resultsBox = new St.BoxLayout({
                vertical: true,
                style_class: 'appmenu-search-results',
            })

EGO-L-003 warning

signals connected by extension should be disconnected in disable()

Signals assigned in `enable()` are missing matching disconnect calls in `disable()` or its helper methods.

Disconnect all signals

  • menuManager.js:178
            this._menuOpenSignalId = this.menu.connect('open-state-changed', (_menu, isOpen) => {
                if (isOpen)
                    this._menuOpenHandler?.();
            })
  • menuManager.js:202
                  subMenu.menu.connect('open-state-changed', (_menu, isOpen) => {
                      if (isOpen)
                          item.onOpen();
                  })
  • searchDialog.js:80
            clutterText.connect('key-press-event', (_actor, event) => {
                const key = event.get_key_symbol();
                if (key === Clutter.KEY_Escape) {
                    this.close();
                    return Clutter.EVENT_STOP;
                }
                if (key === Clutter.KEY_Down) {
           
  • searchDialog.js:79
    clutterText.connect('text-changed', () => this._scheduleSearch())

All Versions

Version Status
6 Unreviewed
5 Rejected
4 Rejected
3 Rejected
2 Rejected
1 Rejected

Previous Reviews on this Version

chathurangaBW auto- rejected
Auto-rejected because of new version 3 was uploaded