Review of "System Monitor" version 26

Details Page Preview

Display resource usage. Linux distribution specific installation instructions can be found in the wiki at https://github.com/elvetemedve/gnome-shell-extension-system-monitor/wiki/Installation. Please report bugs here: https://github.com/elvetemedve/gnome-shell-extension-system-monitor/issues

Extension Homepage
https://github.com/elvetemedve/gnome-shell-extension-system-monitor

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
38 Active
37 Rejected
36 Active
35 Active
34 Active
33 Active
32 Rejected
31 Active
30 Rejected
29 Rejected
28 Active
27 Rejected
26 Active
25 Active
24 Active
23 Active
22 Active
21 Active
20 Active
19 Active
18 Active
17 Active
16 Inactive
15 Rejected
14 Active
13 Active
12 Active
11 Inactive
10 Active
9 Active
8 Active
7 Active
6 Active
5 Active
4 Active
3 Active
2 Active
1 Active

Previous Reviews on this Version

JustPerfection active
`idle_add()` is the same as timeout. Please remove it on destroy for the next version (line 20 helpers/file.js): https://gjs.guide/extensions/review-guidelines/review-guidelines.html#remove-main-loop-sources
elvetemedve posted a review
Thanks for the feedback. The documentation mentioned that returning the value GLib.SOURCE_REMOVE also works fine. So I assumed that what you suggest will apply only when GLib.SOURCE_CONTINUE is returned or there is no return statement. See https://gjs.guide/guides/gjs/asynchronous-programming.html#removing-sources I'm a bit confused now.
JustPerfection posted a review
That link is not related to extensions. As we mentioned on EGO review guidelines: > You MUST remove all active main loop sources in disable(), even if the callback function will eventually return false or GLib.SOURCE_REMOVE. https://gjs.guide/extensions/review-guidelines/review-guidelines.html#remove-main-loop-sources The reason we ask for mainloop removal on disable or destroy is that timeouts can get delayed and triggered after disable. That can lead to accessing destroyed instances or security issues. I approved this one since you used `idle_add` not `timeout_add`. They're usually get triggered very fast and that's why we are not that strict about `idle_add`. If you use `timeout_add` like that, it will get rejected.