-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Port libsoup3 #52
base: master
Are you sure you want to change the base?
Port libsoup3 #52
Conversation
f8a3343
to
a739376
Compare
Both previous issues are fixed. |
This would fix #51 right? |
Would love to see this PR merged :) |
I have been using this as a PKGBUILD for 11 days now, it's has been working flawlessly. I would love to see this merged. |
The only issue is that, if the notifications cannot be grabbed (e.g. the token being invalid, expired or missing, or the device being offline), notifications count will be shown as "0" instead of "!" (i.e. the previous behavior). BTW, good job! Thanks! |
There's also another issue happening here. The notifications count doesn't update at the interval and regularly (e.g. every one minute), but only when I reload the extension or make a login (e.g. after locking the screen). Kinda weird. |
I noticed the same, many times it took a lot to update (0 => N or N => 0...) I didn't dig into it yet.. |
@prahal It appears that the current maintainer is no longer active. I believe it would be beneficial to maintain the forked version. Would you be interested in taking on this responsibility? |
@git-Hmmm I cannot spend the time to maintain a proper fork (published on extensions.gnome.org). Mainly because currently I invest time in a few other projects. But I could maintain an initial code fork on GitHub with an issue tracker. Maybe later on I could publish the extension with proper metadata for a fork on extensions.gnome.org. |
@git-Hmmm also note that a maintained fork already exists. https://github.com/mackdk/gnome-github-manager . Maybe it would be better to check if it supports libsoup3 and else port my patch to it than starting a new fork. |
@prahal Thanks for the info. I will checkout this fork. |
I am currently on gnome shell 43.0 binded to libsoup3 gir on Debian and it works.
It also works on a Debian install with gnome-shell 42.4 binded to libsoup 2.4.
But there are blind spots in the error paths:
I do not know if the err (a GError) is converted to a string properly on error, I guessed.
Also, the 'Body is null' prefix might not be the best.
error('result error: ' + JSON.stringify(response));
to
error('result error: ' + JSON.stringify(result));
I do not know what a GAsyncResult stringified will become. The issue is I do not know how it worked before with a SoupMessage.
So it is in a "works for me" form. As those changes are specific to the libsoup3 code paths maybe it could be merged and those issues handled when those code paths are triggered.