appstream
appstream copied to clipboard
Add an <internet/> kind to requires/recommends/supports
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
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 bandwidth_mbitps attribute, which was more of a question mark in the discussion on #343 than the rest of the new bits in the spec. It should be fairly straightforward to drop bandwidth_mbitps from this MR if you don’t think it’s ready to add that now. It can always be added to the spec in future if people end up thinking that’s a good idea.
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.
I initially thought of dropping the bandwidth_mbitps stuff, but tbh I think it looks fine and I would be okay with adding it now if you/your apps/your client would use it. If it has zero users, then I would be a lot more hesitant with including it.
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 https://github.com/ximion/appstream/pull/425