Add appstream metadata file
ClosedPublic

Authored by kkrawiec on Jan 13 2018, 6:57 PM.

Details

Summary

BUG: 388923

KLines is missing appstream metadata (http://appstream.ubuntu.com/bionic/universe/issues/klines.html). After adding the metadata file, KLines can be available in KDE Discover and GNOME Software.

Diff Detail

Repository
R402 KLines
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kkrawiec created this revision.Jan 13 2018, 6:57 PM
Restricted Application added a reviewer: KDE Games. · View Herald TranscriptJan 13 2018, 6:57 PM
Restricted Application added a subscriber: KDE Games. · View Herald Transcript
kkrawiec requested review of this revision.Jan 13 2018, 6:57 PM

Please remove the translations and fix the cmake file to install this file.

org.kde.klines.appdata.xml
162
ltoscano requested changes to this revision.Jan 13 2018, 7:46 PM
This revision now requires changes to proceed.Jan 13 2018, 7:46 PM
kkrawiec updated this revision to Diff 25285.Jan 13 2018, 9:20 PM

Add suggested changes

kkrawiec added a comment.EditedJan 13 2018, 9:32 PM

Please remove the translations and fix the cmake file to install this file.

Hi!
Thanks for your review. Sorry, but I have zero experience with cmake and I don't know how to fix the file, so it'd nice if somebody else fixes this.

Also, one note: the default screenshot fails the appstream-util validation:

$ appstream-util validate org.kde.klines.appdata.xml 
org.kde.klines.appdata.xml: FAILED:
• style-invalid         : <image> has vertical padding [https://www.kde.org/images/screenshots/klines.png]
• style-invalid         : <image> has horizontal padding [https://www.kde.org/images/screenshots/klines.png]
Validation of files failed

I'd recommend to change it to another one. For example, the attached file passes the test (and its ratio is 16:9. which is on pair with metadata standards).

ngraham edited the summary of this revision. (Show Details)Jan 13 2018, 10:34 PM

I will fix the cmake file after this is fixed. In general, you could take a look at how this is done in other repositories).

The image on kde.org can be fixed by just cropping the empty borders, but I don't have access there.

Final notes: even if not too important, please try to reorder the tags more or less following https://techbase.kde.org/MetaInfo/Components .

org.kde.klines.appdata.xml
3

I'm not a lawyer, but I don't remember this header used too much in the other files.

19

According https://www.freedesktop.org/software/appstream/docs/chap-Metadata.html, "A list of valid category names can be found in the Freedesktop menu spec.", so https://specifications.freedesktop.org/menu-spec/latest/apa.html. Please remove Qt and KDE, which are not categories.

25

As the entire section is none, can you please remove it?

54

For new files, we are trying to use FSFAP: https://spdx.org/licenses/FSFAP.html
Also, please put this tag on the top (see https://techbase.kde.org/MetaInfo/Components )

56

Please remove this; klines is mostly gettext, but also uses many libraries from Frameworks which are either gettext or qt, and Qt is also qt. In general, we didn't define a policy for this tag so far.

kkrawiec updated this revision to Diff 25299.Jan 14 2018, 9:46 AM
kkrawiec marked an inline comment as done.

Add some of the suggested changes

kkrawiec marked 3 inline comments as done.Jan 14 2018, 9:49 AM

Thanks Luigi for your review. I added some inline comments - please check them.

org.kde.klines.appdata.xml
19
25

Removing this section causes a validation error:

org.kde.klines.appdata.xml: FAILED:
• tag-missing           : <content_rating> required for game [use https://odrs.gnome.org/oars]

Are you sure that I should remove it?

56

Ok, I'll remove this. Please note though that it causes validation to fail:

org.kde.klines.appdata.xml: FAILED:
• tag-missing           : <translation> not specified
kkrawiec marked an inline comment as done.Jan 14 2018, 9:52 AM
kkrawiec updated this revision to Diff 25304.Jan 14 2018, 10:48 AM

Fix cmake file

mak accepted this revision.Jan 15 2018, 1:11 AM
mak added a subscriber: mak.

Looks good to me :-)

org.kde.klines.appdata.xml
19

You don't need this section necessarily... It will get pulled from the .desktop file if left out.
That said, it also doesn't hurt to have it, so it's not an issue :-) (It makes the metainfo file more complete, which is nice)

25

This is a bit overly pedantic, I think. I'll need to talk to Richard about that, I guess he enforces that tag in order to ensure the game was age-rated (and the existence of that tag shows that it was).

56

As long as appstreamcli validates it, the file is fine, from its structure. The appstream-util tool is a bit strict, maybe you can get some better idea on what is really important by using its validate-relax option.

kkrawiec retitled this revision from Add appstream metedata file to Add appstream metadata file.Jan 15 2018, 1:54 PM
ltoscano accepted this revision.Jan 16 2018, 6:54 PM

Ok, ok. I'm still not sure about the content_rating (this would be the only program that has it, and I think that it should be added explicitly when needed, not every time), but let's proceed.

org.kde.klines.appdata.xml
19

I just hope that Gnome software or other consumers of appstream won't mark software in the "KDE" category as "only for Plasma".

This revision is now accepted and ready to land.Jan 16 2018, 6:54 PM

Now, the problem is that the email that is returned by arc, used when creating the patch, is not a valid email. Can you please share a valid email address?

ltoscano added a subscriber: aacid.Jan 16 2018, 7:24 PM

@aacid kindly provided the email; @kkrawiec , if you plan to send future patches, please fix the git configuration to reflect your email.

This revision was automatically updated to reflect the committed changes.

@aacid kindly provided the email; @kkrawiec , if you plan to send future patches, please fix the git configuration to reflect your email.

Oh, sorry for that. I'll fix that before sending future patches.