gpouilloux / gnome-shell-extension-docker

An extension for managing docker containers
GNU General Public License v3.0
146 stars 71 forks source link

Cherry pick status menu item feature #53

Closed alessandrodolci closed 4 years ago

alessandrodolci commented 4 years ago

Hi @gpouilloux, I extracted the Docker daemon status feature from the PR you linked me, it seems to be working without any particular issue. I tested it on gnome-shell 3.34 and 3.36, on Ubuntu 19.10 and 20.04 respectively (I updated the os yesterday, so I had the chance to try this on both the newest and the previous version).

I took the opportunity to update the versions list in the metadata.json file, as well as to give a tiny refresh to the readme file, I hope you're ok with this.

I also noticed two different warnings in the gnome logs, about two different library deprecations: one is about the output of some GObject functions moving from a custom type to JS Uint8Array built-in type (as reported here: https://gitlab.gnome.org/GNOME/gjs/-/tags/1.53.90); the other one, which I think is more recent (maybe came out with 3.34), complains about accessing the actor property on UI GObject instances, which we do inside DockerMenu and DockerSubMenuMenuItem. I tried two easy fixes which seem to work well, hopefully even maintaining compatibility with older versions, but I haven't included them in this PR, because I thought it would have been better to report the problems first. I can commit them here, or open a specific issue and/or a new PR, based on what you think would be better to keep track of the changes.

Tell me what you think, as always I'll stick around for any help or clarifications :+1:

gpouilloux commented 4 years ago

@alessandrodolci

I also noticed two different warnings in the gnome logs

Good catch about these! I'd suggest creating a dedicated GitHub issue to report them & fix them with a very different PR to ensure traceability. Thanks again!

alessandrodolci commented 4 years ago

I commited a refactoring of the class, take all the time to review it.

I tried to let it reflect more the structure of DockerMenuItem, since their behavior is somewhat similar.

I also noticed that error handling on the on/off action dispatch wasn't quite right, so I went on to fix it. Basically, it was assigning the result of spawn_command_line_async (a boolean) to [res, out, err, status] and then using those variables to do the logging. Sorry for missing this one before, my bad.

@alessandrodolci

I also noticed two different warnings in the gnome logs

Good catch about these! I'd suggest creating a dedicated GitHub issue to report them & fix them with a very different PR to ensure traceability. Thanks again!

Perfect, I'll proceed with that as soon as possible!

Thank you for all the feedback!