Review of "Media Label and Controls (Mpris Label)" version 42

Details Page Preview

Display a label in your panel with the song/title/album/artist information available from an mpris compatible player. You can also control the player, raise/lower its volume, customize the label, and a lot more! This extension works with Spotify, Vlc, Rhythmbox, Firefox, Chromium, and (probably) any MPRIS compatible player.

Extension Homepage
https://github.com/Moon-0xff/gnome-mpris-label

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 5

Shexli found 5 issues that may need reviewer attention.

EGO-X-006 warning

extensions should not use lookupByURL or lookupByUUID for current extension access

Use `this`, `this.getSettings()` or `this.path` instead of `lookupByURL()` or `lookupByUUID()` for the current extension.

`extensionUtils`

  • extension.js:440
    Extension.lookupByUUID('mprisLabel@moon-0xff.github.com')

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-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

  • extension.js:68
    this.settings.connect('changed::extension-index',() => {this._updateTrayPositionPending = true;})
  • extension.js:69
    this.settings.connect('changed::extension-place',() => {this._updateTrayPositionPending = true;})
  • extension.js:73
    this.settings.connect('changed::font-color', this._setLabelStyle.bind(this))
  • extension.js:65
    this.settings.connect('changed::left-padding',this._onPaddingChanged.bind(this))
  • extension.js:66
    this.settings.connect('changed::right-padding',this._onPaddingChanged.bind(this))
  • extension.js:70
    this.settings.connect('changed::show-icon',this._setIcon.bind(this))
  • extension.js:72
    this.settings.connect('changed::symbolic-source-icon', this._setIcon.bind(this))
  • extension.js:71
    this.settings.connect('changed::use-album',this._setIcon.bind(this))
  • extension.js:62
    this.volumeControl.connect("stream-added", this._getStream.bind(this))
  • extension.js:63
    this.volumeControl.connect("stream-removed",this._getStream.bind(this))
  • players.js:172
    this.proxy.connect('g-properties-changed', this.update.bind(this))

EGO-L-004 warning

main loop sources should be removed in disable()

Main loop sources assigned in `enable()` are missing matching removals in `disable()` or its helper methods.

Remove main loop sources

  • extension.js:77
    this._repositionTimeout = GLib.timeout_add_seconds(GLib.PRIORITY_DEFAULT,REPOSITION_DELAY,() => {this._updateTrayPositionPending = true;})
  • extension.js:167
    		this._scheduledActionTimeout = GLib.timeout_add(GLib.PRIORITY_DEFAULT, DOUBLE_CLICK_TIME, () => {
    				this._activateButtonAction(button,false);
    				return GLib.SOURCE_REMOVE; // callback function will be executed once
    			}
    		)
  • extension.js:467
    			this._timeout = GLib.timeout_add(GLib.PRIORITY_DEFAULT,
    				REFRESH_RATE, this._refresh.bind(this))

EGO-L-007 warning

main loop sources should be removed before being recreated

Main loop sources should be removed before creating a new source on the same field.

Remove main loop sources

  • extension.js:484
    		this._timeout = GLib.timeout_add(GLib.PRIORITY_DEFAULT,
    			REFRESH_RATE, this._refresh.bind(this))

All Versions

Version Status
42 Rejected
41 Active
40 Active
39 Active
38 Active
37 Active
36 Active
35 Active
34 Active
33 Active
32 Active
31 Active
30 Active
29 Active
28 Active
27 Active
26 Active
25 Active
24 Rejected
23 Active
22 Rejected
21 Rejected
20 Rejected
19 Rejected
18 Active
17 Active
16 Active
15 Active
14 Active
13 Active
12 Active
11 Rejected
10 Active
9 Rejected
8 Rejected
7 Active
6 Rejected
5 Rejected
4 Active
3 Active
2 Rejected
1 Active

Previous Reviews on this Version

JustPerfection rejected
1. Please remove `schemas/gschemas.compiled`. Not needed for 45+ packages. 2. `lookupByUUID()` is a bad practice (line 658 `extension.js`). You can send `this` from the entry point to the class needing it when it's possible (dependency injection).