Review of "Workspace Wallpapers" version 3

Details Page Preview

Assign a different wallpaper to each workspace. The correct background is visible on every workspace panel during the slide animation and in the overview — not just after the transition completes.

Extension Homepage
https://github.com/corjix/gnome-workspace-wallpapers

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 1

Shexli found 1 issue that may need reviewer attention.

EGO-L-006 warning

preferences classes should not retain window-scoped objects on instance fields without close-request cleanup

Preferences code stores window-scoped objects on the exported prefs class without `close-request` cleanup.

Destroy all objects

  • prefs.js:29
    this._wmSettings = new Gio.Settings({schema_id: 'org.gnome.desktop.wm.preferences'})
  • prefs.js:30
    this._mutterSettings = new Gio.Settings({schema_id: 'org.gnome.mutter'})

All Versions

Version Status
4 Unreviewed
3 Rejected
2 Active
1 Active

Previous Reviews on this Version

JustPerfection rejected
Please don't store any instance of objects in a property in the default class you are exporting in `prefs.js`. That won't allow the garbage collector to do its job after window close: [EGO Review Guidelines: Destroy](https://gjs.guide/extensions/review-guidelines/review-guidelines.html#destroy-all-objects) If you don't want to move them to another class or make it local to `fillPreferencesWindow()` function, you can clean up on: ```js window.connect('close-request', () => { // clean up here }); ```
corjix posted a review
I pushed a new version addressing your comments.