Skip to content

Comments

Get screenshots from debian.net in Arch#75

Merged
ximion merged 1 commit intoximion:masterfrom
antonio-rojas:master
Sep 8, 2016
Merged

Get screenshots from debian.net in Arch#75
ximion merged 1 commit intoximion:masterfrom
antonio-rojas:master

Conversation

@antonio-rojas
Copy link
Contributor

No description provided.

@ximion
Copy link
Owner

ximion commented Sep 8, 2016

Merging this one, but there is a high chance that I will drop this feature and make appstream-generator cache the screenshots instead.
Software center authors complained about getting a dummy picture when the screenshot isn't available, and for querying whether it is available and then not-showing an image, we would need extra API specifically for this service.
Having the screenshots fetched in asgen will solve all of these issues.

@ximion ximion merged commit 4e40518 into ximion:master Sep 8, 2016
@aleixpol
Copy link
Collaborator

aleixpol commented Sep 8, 2016

Are we sure this is going to work? Does debian screenshots support appstream ids now?

@ximion
Copy link
Owner

ximion commented Sep 8, 2016

This will only work if the Arch package names are equal to the Debian package names.
Since this is, well, a Debian service.
(that is also the reason why this is configurable, since some distros run or ran own versions of the screenshots.d.n software, or imitate its API)

@aleixpol
Copy link
Collaborator

Then this sounds like a horrible idea... we already went through this, it didn't work well.

@antonio-rojas
Copy link
Contributor Author

I must be missing something here... How is downloading screenshots from s.d.net worse than not having any screenshot at all (for packages that don't provide it themselves)?

@aleixpol
Copy link
Collaborator

By default we used to have this, and Discover UI was full of images saying image not found. Probably because they don't really give a 404 error when the image wasn't found.

@Signum
Copy link

Signum commented Sep 12, 2016

Just to let you know. I'm following this thread if you have any ideas, wishes or requests. I intend to implement AppStream parsing to get the data soon. I'm just still confused whether I need to implement both XML and YAML parsing. The screenshot's site runs on Ruby (on Rails) and there doesn't seem to be any real Ruby Gem for that purpose yet. So I need to use either your command-line tools or implement the parser(s) myself.

…Christoph (the guy behind screenshots.debian.net)

@antonio-rojas
Copy link
Contributor Author

And without this patch you get only the "missing image" icon instead of any screenshot, which looks even more broken. So again, I don't see how this makes anything worse.

@ximion
Copy link
Owner

ximion commented Sep 12, 2016

@Signum I am planning to go the other way round and use screenshots.d.o to complement the AppStream metadata when we generate it for Debian, using its JSON interface - for that it would be useful to return a consistent error code when something wasn't found, at time that could be a 404 code, some page or a JSON document saying that no screenshot was found.

Whether AppStream is XML or YAML depends on whether you want to read metainfo files (always XML) or collection files (YAML or XML). In any case, I recommend using libappstream for reading stuff, if Ruby understands GIR that should be pretty trivial.

ximion added a commit that referenced this pull request Sep 16, 2016
@ximion
Copy link
Owner

ximion commented Sep 17, 2016

I reverted the patch due to its controversial nature, and because the AppStream-Generator will itself soon pull the proper files and set the right metadata, rendering this feature in the AppStream client tool obsolete.

Patches for asgen to implement the screenshots.d.o feature are very welcome, as that doesn't have the highest priority at the moment.

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.

4 participants