Add an <internet/> kind to requires/recommends/supports #421
Conversation
This allows apps to specify whether they require the internet to work, or whether they explicitly work fully without internet. Signed-off-by: Philip Withnall <[email protected]> Fixes: ximion#343
…kind Includes tests for the XML and YAML implementations. Signed-off-by: Philip Withnall <[email protected]> Fixes: ximion#343
|
I’m going to be away for a few months from basically now, so won’t be able to respond to review feedback on this after tomorrow or so. Please feel free to update/change/drop/close/adapt the MR in whatever way seems most appropriate. One thing you might want to think about in particular is the |
| </para> | ||
| <itemizedlist> | ||
| <listitem><para><code>always</code> - Needs internet connectivity to work. If used in a <literal>recommends</literal> element, then this indicates that the app can work without internet, but the experience will be significantly degraded.</para></listitem> | ||
| <listitem><para><code>offline-only</code> - Never uses the internet, even if it’s available.</para></listitem> |
There was a problem hiding this comment.
What is the value in case an application can use the internet to enhance the experience, but otherwise does not need to and would also work perfectly fine offline? (e.g. a media player fetching cover art for songs from the web, but which would play them perfectly find just as well w/o internet connection)
Would that be maybe:
<supports>
<internet>always</internet>
</supports>| @@ -1044,6 +1184,16 @@ as_relation_load_from_xml (AsRelation *relation, AsContext *ctx, xmlNode *node, | |||
| g_autofree gchar *side_str = (gchar*) xmlGetProp (node, (xmlChar*) "side"); | |||
| priv->display_side_kind = as_display_side_kind_from_string (side_str); | |||
|
|
|||
| g_free (priv->version); | |||
There was a problem hiding this comment.
I know it's not your code, but what about g_free (g_steal_pointer (&priv->version)) or as_relation_set_version (relation, NULL)?
| g_free (priv->version); | ||
| priv->version = NULL; | ||
| } else if (priv->item_kind == AS_RELATION_ITEM_KIND_INTERNET) { | ||
| g_autofree gchar *bandwidth_str = (gchar*) xmlGetProp (node, (xmlChar*) "bandwidth_mbitps"); |
There was a problem hiding this comment.
g_autofree gchar *bandwidth_str = as_xml_get_prop_value (node, "bandwidth_mbitps");
(a bit nicer to read)
| @@ -1101,6 +1251,11 @@ as_relation_to_xml_node (AsRelation *relation, AsContext *ctx, xmlNode *root) | |||
| (xmlChar*) as_relation_item_kind_to_string (priv->item_kind), | |||
| (xmlChar*) as_control_kind_to_string (as_relation_get_value_control_kind (relation))); | |||
|
|
|||
| } else if (priv->item_kind == AS_RELATION_ITEM_KIND_INTERNET) { | |||
| n = xmlNewTextChild (root, NULL, | |||
There was a problem hiding this comment.
You could use as_xml_add_text_node here, but that's optional :-)
|
Looks really good, all the comments I had are very minor. The only thing I think we need to answer is the "app that can use internet but doesn't need to" case. All in all, thank you a lot for the really good patch and sorry for the delayed reply as well as causing a merge conflict by converting the AsRelation code to using the XML helpers ;-) (they don't do much except for making the C code easier to read). |
|
You should probably explicit mention that using it multiple times is supported. Something like this: <requires>
<internet>first-run</internet>
</requires>
<recommends>
<internet>always</internet>
</recommends>This is to avoid confusion in the implementations. |
|
Closing in favour of #425 |
See the commit messages. This adds a new
<internet/>kind to requires/recommends/supports, both in the spec and in libappstream. It includes unit tests.Fixes: #343