Fix: #378 Issue non-fatal warnings on missing metadata

[rhl-bthr]

Fixed in this PR: #378 Changes made:

• python setup.py dist_xo will now issue warnings(non-fatal), if activity.info won't have license or activity_version tags
• name tag already issued a fatal error, if missing and hence was excluded.

Suggestion: All tags independent of whether they are necessary for aslo should issue a warning. Kindly let me know, I will update the PR

Tested

[quozl]

You've added to the XOPackager class. I hadn't thought of that.

Better would be to check config in cmd_dist_xo, as at that point the file is already parsed and available.

Another place would be class ActivityBundle which will help with aslo-v3 workflow which (at one stage) proposed not to require use of the dist_xo target by developers.

Regarding what tags must be present;

• compare against our toolkit's authoritative specification of metadata keys.
• search the in-production aslo repository for more details, see https://github.com/sugarlabs/aslo/search?utf8=%E2%9C%93&q=activity_version&type=
• search the in-development aslo-v3 repository for more details, see https://github.com/sugarlabs/aslo-v3/search?utf8=%E2%9C%93&q=activity_version&type=
• see our Wiki specification of the file format.

[rhl-bthr]

Thank you for the detailed review and guidance

Update:

• Decided to go with ActivityBundle, as it might help with aslo-v3 workflow in future
• Displaying warnings only for the mandatory tags and not the optional ones
• Also displaying warnings for tags mandatory for AppStream

Sources for looking up tags: (Thanks to @quozl)

• ASLO release.py L141
• /Bundle/init.py
• Wiki

[quozl]

Thanks. Reviewed.

• [x] there is a check for name, bundle_id and some other keys already in _parse_info; let's not add redundant checks,
• [x] does not seem to be a good reason for a new method of the class, especially also as the method is not privately named; place it inline in _parse_info instead,
• [x] other messages use "does not" rather than "doesn't"; please keep the messages consistent,

I'm not so sure about the AppStream changes, as there are almost no activities that have AppStream metadata (e.g. only Browse), so we would end up with lots of warnings in logs as Sugar starts up for no real benefit. I don't think anybody is using our AppStream metadata anyway. Anyway, I did notice;

• [x] Appstream should be AppStream to be consistent with other uses,
• [x] some of the your AppStream additions become redundant with _generate_appdata in bundlebuilder.py,

I've also updated the specification https://github.com/sugarlabs/sugar-toolkit-gtk3/commit/7e216a04d579d6f0e8283e5d3975bf72edd2f67b.

[rhl-bthr]

@quozl Thanks once again for the detailed review

Changes made:

• [X] Removed redundant checks already checked in _parse_info
• [X] placed the checks inline in _parse_info, rather than a new method
• [X] Changed warning message, to maintain consistency
• [X] Checks for AppStream metadata removed, considering that there are redundant checks and the

Changes in 7e216a0 are for optional metadata. Kindly let me know if checks for optional metadata should also be added.

Kindly review If no more changes are to be made, shall I squash all commits.

[quozl]

Thanks. Reviewed.

Rebased. Rewrote messages for consistency with others. Moved some checks into else clauses where the same metadata key was tested.

https://github.com/sugarlabs/sugar-toolkit-gtk3/commit/cc2357c787cb30d02e1568a2bf23dc18c367468f