Closed francoism90 closed 6 months ago
Started test build 86264
Build 86264 failed
Started test build 86276
Build 86276 successful To test this build, install it from the testing repository:
flatpak install --user https://dl.flathub.org/build-repo/68937/com.synology.SynologyDrive.flatpakref
Started test build 86556
Build 86556 failed
Started test build 86558
Build 86558 successful To test this build, install it from the testing repository:
flatpak install --user https://dl.flathub.org/build-repo/69218/com.synology.SynologyDrive.flatpakref
Started test build 86559
Build 86559 successful To test this build, install it from the testing repository:
flatpak install --user https://dl.flathub.org/build-repo/69219/com.synology.SynologyDrive.flatpakref
Started test build 86561
Build 86561 successful To test this build, install it from the testing repository:
flatpak install --user https://dl.flathub.org/build-repo/69221/com.synology.SynologyDrive.flatpakref
Started test build 86566
Build 86566 successful To test this build, install it from the testing repository:
flatpak install --user https://dl.flathub.org/build-repo/69226/com.synology.SynologyDrive.flatpakref
Started test build 86569
Build 86569 successful To test this build, install it from the testing repository:
flatpak install --user https://dl.flathub.org/build-repo/69229/com.synology.SynologyDrive.flatpakref
Thank you for your contribution! I think we will want to update to the newest version in another commit and then look at all the small incremental improvements separately, what do you think @lil5? Could you maybe also explain the additional permissions etc. you added in this PR?
To keep testing quality high keep it to this PR, but an explanation on each change is required.
@RayJW @lil5 Thanks for your reply and review. :)
I'm still relatively new to Flatpak, so if anything should be changed/undo, please let me know.
Changes:
--own-name=org.kde.*
, should be removed. It should work with org.kde.StatusNotifierWatcher
--system-talk-name=org.freedesktop.login1
is needed, so the application can track the user. If this is unneeded, I'll remove it.--talk-name=org.gnome.SessionManager
, I think the application use this, but I'm not sure.$version
- $version4
, unless the 4 has a meaning?I believe the
--own-name=org.kde.*
, should be removed. It should work withorg.kde.StatusNotifierWatcher
That checks out. We actually wanted to make that change already, but we don't get around to testing a lot, since neither of us actually needs this software as far as I am aware. Did you test if the tray icon still properly works after that change?
- I think
--system-talk-name=org.freedesktop.login1
is needed, so the application can track the user. If this is unneeded, I'll remove it.
Track in what way? The only use case I saw for this permission so far is to enable start on login functionality, does the client provide that functionality and does it work with this permission enabled?
--talk-name=org.gnome.SessionManager
, I think the application use this, but I'm not sure.
Use for what exactly? I think so far it worked great without, or is there a function that was broken for you without it? It's a possibility for sure since the only tests we can do is by hand, so not all functionality is tested as thorough!
- I've fixed the url
$version
-$version4
, unless the 4 has a meaning?
The 4 does indeed have a meaning, it's for the fourth part of the version regex, since the URL doesn't contain the whole version identifier. However, it was broken due to another issue, which I fixed upstream. This should work again, once you rebase with main :)
@RayJW Thanks for your review again, and the explanation about the flags. :)
I'see other PRs are open already, also on for bumping this package. Should I close this one? :)
No, I think we keep your version. The other PR is just automatically made by the bot. If you can confirm that the new flags enable new functionality according to your intention and remove all the unneeded changes, I think we can merge this. Agreed, @lil5?
@RayJW Maybe the flags aren't needed.
If you would like me to remove some, please let me know. :)
Started test build 86998
Build 86998 successful To test this build, install it from the testing repository:
flatpak install --user https://dl.flathub.org/build-repo/69658/com.synology.SynologyDrive.flatpakref
Started test build 87000
Build 87000 successful To test this build, install it from the testing repository:
flatpak install --user https://dl.flathub.org/build-repo/69660/com.synology.SynologyDrive.flatpakref
So, I just tested your build and for me personally neither start on boot seemed to work nor did the background portal seem to change anything since it seems to work fine. Also, the tray icon doesn't work any more with the removed permissions, so that doesn't seem to work out either.
Were those things working on your end when you tested those changes? It could still be a my machine(tm) issue.
I also tested the new build from the other PR and there the tray icon still works, so it does not seem to be a regression in the new software version.
@RayJW Thanks for testing. :)
I'll close this PR, as it's incomplete, and I think the other PRs are better.