search

0ry0n / Resource_Monitor

Resource_Monitor is a GNOME Shell extension that Monitor the use of system resources like cpu, ram, disk, network and display them in GNOME Shell top bar.
https://extensions.gnome.org/extension/1634/resource-monitor/
GNU General Public License v3.0
154 stars 21 forks source link

Port to Gnome 45: initial efforts #78

Closed Sierra410 closed 12 months ago

Sierra410 commented 1 year ago

This makes it work on Gnome 45, but it's not perfect as of now.

I am fairly sure this breaks backwards compatibility entirely, which is a very major issue that has to be solved.

After consulting with people in #extensions:gnome.org, it appears that the solution to backwards compatibility, is not doing backwards compatibility.
Users will fetch the latest version of the extension their shell supports, so, codebases for 45+ and <44 should live in two different branches and be maintained, packaged, and uploaded separately.

~~Imports cannot be wrapped around try-catch, rendering gi://NM a hard dependency, at this point.
I'm not sure what's the best way to re-implement the old behavior (or if there is one, for that matter), so that's something that needs to be looked into.~~

Fixed. Courtesy of @Braayy.

~~Certain values are made accessible to class instances via global variables (those being extension in extenion.js; and gettext_domain, settings, and dir in prefs.js). This works, as a proof of concept, but needs to be implemented properly.
I am very much unfamiliar with this codebase, so I tried to keep things as intact as possible, hence this workaround.~~

Replaced globals with dependency injection.
I think in this case the proper solution would be to re-factor the code, but I don't really feel comfortable doing that.

Other changes: Legacy ByteArray has been replaced with TextDecoder. global.log has been replaces with log, because global.log does not exist.

ezequielramos commented 1 year ago

@0ry0n

0ry0n commented 1 year ago

In the coming days I will start working on the new version, but these changes seem ok. Thanks for your cooperation.

Sierra410 commented 12 months ago

log() should be replaced with console.* family of functions, actually. https://gjs.guide/extensions/upgrading/gnome-shell-45.html#logging

While log() works, EGO review team points it out as something that should be fixed.