Skip to content
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

feat: allow updating AppImages from AppImageHub.com #178

Merged
merged 1 commit into from
Nov 10, 2021

Conversation

azubieta
Copy link
Contributor

No description provided.

@probonopd
Copy link
Member

probonopd commented Oct 23, 2021

Thanks @azubieta, this would be a great addition. Let's see that we can get it to build.
The build error seems unrelated to your code:

This request was automatically failed because there were no enabled runners online to process the request for more than 1 days.

Triggered a re-run, let's see how it goes.

Getting now

Can't find any online and idle self-hosted runner in the current repository, account/organization that matches the required labels: 'ubuntu-16.04'
Waiting for a self-hosted runner to pickup this job...

What is going on... turns out GitHub removed Ubuntu 16.04 from their default templates because it no longer receives standard support from Canonical:

#179

@azubieta azubieta force-pushed the feat/support_updates_form_appimagehub branch 2 times, most recently from caccb94 to 79a0c0f Compare October 25, 2021 22:12
@probonopd
Copy link
Member

probonopd commented Oct 26, 2021

Would it be possible to rename "pling" to "OCS" and remove the hardcoded URL from the source to make it more generic?

Looking at AppImage/AppImageKit#1152 (comment) the URL is part of the proposed update information string.

@azubieta azubieta force-pushed the feat/support_updates_form_appimagehub branch from fcbb0ee to 7554bb9 Compare October 26, 2021 04:50
@azubieta
Copy link
Contributor Author

The original plan was to support the OCS API but they guys from pling did some modifications to it. Like generating a zsync file for each download. I don't think this would be available in other implementations.

I'll update the related issue in the AppImageKit repo.

@probonopd
Copy link
Member

probonopd commented Oct 26, 2021

Then we need a pling update information type. Still would make the URL not hardcoded, as someone might set up their own pling instance (I guess).

@azubieta azubieta marked this pull request as ready for review October 26, 2021 17:29
@probonopd probonopd requested a review from TheAssassin October 29, 2021 20:19
@azubieta
Copy link
Contributor Author

azubieta commented Nov 1, 2021

Then we need a pling update information type. Still would make the URL not hardcoded, as someone might set up their own pling instance (I guess).

I'm no sure if hosting a pling instance is something someone could do as the code is not published anywhere.

@azubieta azubieta merged commit 97645f5 into main Nov 10, 2021
@azubieta azubieta deleted the feat/support_updates_form_appimagehub branch December 2, 2021 19:25
@TheAssassin
Copy link
Member

TheAssassin commented Jan 4, 2022

This PR has not passed a QA review, and therefore there are multiple issues, for instance there's no clear interface in the new update methods concept (abstract base class, for instance) that could be programmed against, thus the updater implementation needs to know implementation details.

Of course, the lack of abstractions is a general problem in AppImageUpdate, my original design is anything but ideal by my current standards. I think an opportunity to fix some of the issues in the code base was missed here, though. I'm reworking parts of the updater anyway to implement a couple new features in libappimageupdate that'll make integration in third-party software easier (which I had wanted to do for a while already, there is a feature wishlist), and in this process I'm going to fix those issues. I already introduced a separate type for the update information (similar

I'd highly appreciate waiting for a qualified review, it really helps increase code quality. 10 days is really not a long time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants