GNOME Bugzilla – Bug 722763
Add AppData file
Last modified: 2018-03-23 15:49:11 UTC
Created attachment 266967 [details] [review] Patch to add AppData file Patch to add AppData file, with text suggested by Hughsie.
Created attachment 266971 [details] [review] Patch updated Added missing reference in POTFILES.in
Ping?
Review of attachment 266971 [details] [review]: I'm not a yelp developer, but let's try and get this in. The patch looks good to me, except for a few minor things: ::: data/yelp.appdata.xml.in @@ +2,3 @@ +<application> +<id type="desktop">yelp.desktop</id> +<licence>CC0</licence> licence has gotten deprecated in the mean time while the patch has been lingering here and we now have two new tags instead: metadata_license and project_license. Please update the patch to use those instead. Also, would be great if you could add a comment with copyright information to the top of the file, like <!-- Copyright 2014 Your Name --> @@ +12,3 @@ +</description> +<screenshots> +<screenshot type="default">https://wiki.gnome.org/Apps/Yelp?action=AttachFile&do=view&target=yelp_screenshot.png</screenshot> This is XML; need to escape "&" symbols as "&". Also, the link is wrong: it currently points to a html page, but should point directly to a png image.
Created attachment 283975 [details] [review] Patch updated Thanks Kalev for your review. Here is an up-to-date patch.
getting this in would be great
Review of attachment 283975 [details] [review]: Sorry Daniel, I missed the updated patch. The actual appdata.xml.in file seems to have gone missing from the patch, can you attach that too please?
Created attachment 285833 [details] [review] Patch updated My fail. Forgot to add the new file into git. Here is the updated patch.
Review of attachment 285833 [details] [review]: ::: data/yelp.appdata.xml.in @@ +5,3 @@ + modify it under the terms of the GNU General Public License as + published by the Free Software Foundation; either version 2 of the + License, or (at your option) any later version. The GPL license header is only useful if this file is licensed under the GPL, but later it is claimed to be under CC0. @@ +19,3 @@ +<application> +<id type="desktop">yelp.desktop</id> +<metadata_license>CC0</metadata_license> I think that the correct SPDX license tag is CC0-1.0. @@ +20,3 @@ +<id type="desktop">yelp.desktop</id> +<metadata_license>CC0</metadata_license> +<project_license>CC0</project_license> The proejct is licensed under the GPLv2+, not CC0.
Created attachment 286020 [details] [review] Licenses fixed You are right. I made it looking another bug, and get confused with the licenses. I've updated both licenses fields, using the SPDX license ID's. Thanks for the feedback!
Review of attachment 286020 [details] [review]: Looks OK to me, although I would probably wrap the AppData lines at 79 characters. Also, this would add translatable strings, and so is a bit late for this cycle (although there could be a string freeze break request). Finally, the description is a bit developer-centric (talking about other yelp modules), and something more suited to users would be better, for example: Yelp is the GNOME help viewer. It is the default Mallard viewer, but it can also display DocBook, info, man, and HTML documentation. It makes it easy to find the documentation you need, with interactive search and bookmarks. It also has an editor mode, which shows editorial comments and revision statuses when editing Mallard documents.
String freeze should not be a problem (we already have a +1 from i18n ;-) ), but I'd really like to ask you to do a final review. Once we had the licenses fixed, you propose a new change in the description, which implies to modify and generate (again) a new patch, review it, etc. Shouldn't be easier to all to review all the terms once and comment them altogether? If patch is now ok, I would commit it (asking for a freeze break before, of course), and if, in the future, maintainer considers that the text must be changed, he can do it, but let 's get this issue ;-)
I am not a Yelp maintainer, so what I mentioned should only be taken as a suggestion, nothing more. I think that, if a string freeze break is requested, it is important to make sure the strings are unlikely to be changed again in the near future, as that wastes translator effort.
> I think that, if a string freeze break is requested, it is important to make > sure the strings are unlikely to be changed again in the near future, as that > wastes translator effort. Agree with you. In this case, it would be great to hear Shaun's opinion. Shaun: could you provide us some feedback about this issue? Thanks!
Yelp will be one of only two GNOME modules without AppData in 3.14.0 :(
Honestly, I'd been trying to downplay the idea of Yelp as an app, and make it just be a utility window that happens to be provided by another program. That's been increasingly difficult with the way we've crafted the platform over the 3.x cycle. That's why this bug report has languished.
Can we add NoDisplay=true to the .desktop file then? It's either an app, or it isn't.
Having an app show up in gnome-software, only to hide from the overview after installation is not right. If it is an app, it show up in both places.
Ping, any chance this could land, please? We are currently shipping the appdata file downstream, which means no translations for it ...
And a ping again. Cleaning up old gnome goals...
Ping again... near to close the goal, but this is blocking us ;-)
Ping again - maybe for GNOME 3.30 we manage?
I went ahead and pushed this to git with my gnome-software developer hat on. Downstreams are all shipping appdata patches for yelp and it makes sense to have it upstream here where it can get correctly translated and updated when necessary. I've discussed the strings changes on IRC in #i18n and the consensus was that this is suitable for 3.28.1 as it's fixing untranslated strings in gnome-software. https://git.gnome.org/browse/yelp/commit/?id=92645b012abd631a2356be74f9522ee187a0f46b