For Kodi code we have switched to the SPDX-License-Identifier which makes it easier to identify what licenses are used. Perhaps this is a good idea as well for add-ons. Just as a suggestion for the dev but not make it mandatory.

Some accidentally set wrong file permissions while submitting

Checkstyle is an XML output format for source-code checkers. This will provide interoperability with both IDEs and editors that expect code suggestions in checkstyle format, as well as other continuous code quality platforms (e.g. personal instances of sonarqube). Checkstyle output format is built into many linters found out there (see for instance gometalinter: https://github.com/alecthomas/gometalinter#checkstyle-xml-format).

Exposing found issues in Kodi addons using a common standard will certainly help the development of plugins for most of existing IDEs.

This is not a high priority requirement, just food for though. At least keep the idea in mind while developing the tool further so it will not require drastic architectural changes if someone wants to implement this.
Cheers

I have never done a pip package so far, but it might work for us if we can use pipenv to run this.

We Should have a check for init.py file.
The main reason is that When we run pylint on any folder it first checks for init.py file in it. If the file is missing then it produce an error

This is the error:

This is a know problem in pylint.

Some have "License.txt" or whatever

Just an idea, we're currently blacklisting file endings and filenames, I feel like it might make sense to whitelist instead.

The old format should only be used in gotham or earlier. Instead we should encourage using the new one. See our docs here:

https://kodi.wiki/view/Service_add-ons

Somehow I forgot to include a license 🤦‍♂️

Since the add-on is already marked broken we might as well skip the rest of the errors. So see if broken tag is in addon.xml and skip the rest.

Since several version we support a new settings.xml format which is the same as Kodi itself uses.
As most bugs should have been solved by now we should suggest developers to start using this new layout.
I'm sure there is already some wiki page or forum post explaining how the new layout works

Some sign in the table seem to break the report we get back to travis buddy.

Here's an example:

xbmc/repo-plugins#1976

The complete errors can be found here:
https://travis-ci.org/xbmc/repo-plugins/builds/412547630

So either we fix that somehow or we get rid of the table.

Sometimes developers add circular dependencies by accident which freaks out Kodi. Doesn't happen often but it's quite nasty and we had lots of problems with this since add-ons where added to XBMC

<addon id="plugin.video.fattoquotidianotv" name="Il Fatto Quotidiano TV" version="1.0.4" provider-name="NeverWise"> <requires> <import addon="xbmc.python" version="2.14.0"/> <import addon="script.module.neverwise" version="1.0.6"/> <import addon="plugin.video.youtube" version="4.4.10" optional="true"/> <import addon="plugin.video.serviziopubblico" version="1.0.6" optional="true"/> </requires>

<addon id="plugin.video.serviziopubblico" name="Servizio Pubblico" version="1.0.6" provider-name="NeverWise"> <requires> <import addon="xbmc.python" version="2.14.0"/> <import addon="script.module.neverwise" version="1.0.6"/> <import addon="plugin.video.fattoquotidianotv" version="1.0.4" optional="true"/> </requires>

Take any addon.xml. Change the header from this:

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>

to these:

<?xml version="1.0.1" encoding="UTF-8" standalone="yes"?>

That used to break the repo update script.

Needs to be searched for in v18+ and reported as it's deprecated in Leia.

This issue is a suggestion.

I have seen many cases in which a user submits an addon to a repository (lets say Krypton) and the addon is already present in "lower" repositories (e.g. helix). In those cases the check-tool should produce a warning to inform the user of this fact since most of the time the user is either not aware of the presence of the addon in a lower repo, doesn't know recent versions still have access to those lower repos, the changes are so minimal that do not justify a full addon review or the changes are not version specific.
Finding if the addon already exists in another repo is both a memory exercise or a manual checking step (time consuming) and preferably should be avoided.

When there are issues found it would be nice they would be automatically added as comment on the PR. If need accompanied with url to some helpful page.

We will never be able to find these addons in any repo.

So let's ignore all of these:
https://github.com/xbmc/xbmc/blob/2f9feb2/xbmc/addons/AddonInfo.cpp#L39-L77

This is a generalization of @Rechi's comment here.

I was wondering if we should add a checker that detects whether the addon code is Python 3 compatible?

A very simple way is to just run 2to3 on the code and see if it detects any issues. We could try out other tools like modernize etc. as well. Even though these tools mostly detect if any cosmetic changes are required. They serve as a good first step towards python 3 compatibility.

We could log their output as warning or even suggest changes like use print() rather than print inline as a comment on the pull request itself.

An extension to this would be to just run 2to3 on the code that has been touched in the pull request, rather than running it on the entire file.

Please let me know if this sounds reasonable and I can start working on a pull for this!

Maybe it's possible to check the included dependencies in addon.xml against the repo it's PRed to.
For this you could parse the repo addons.xml (or first download/unzip the addons.xml.gz)

Over time some add-ons might become broken/obsolete and will be removed. Some have a certain helper add-on which we then also forget to remove. Would be nice if we could detect this in some way if there are any. This can be done in the general scan on each branch and doesn't need to bother the regular submission report.

A common issue when people submit addons to the repository is the presence of a .git folder, a .gitignore or .gitattributes file in the PR. The addon-check tool should look for the presence of such files and automatically inform the user to remove them from the pull request.

Ref: xbmc/repo-plugins#2001

script.module.inputstreamhelper is an optional dependency of the youtube addon and only makes sense for v17+. Youtube is in the isengard repo and is able to operate even without inputstream.

Question is...should we report such cases as problem? IMO warning fits better if the dependency is optional.

Resource addons don't name .py files but .xml files and thus radon can't parse them.

We should ignore those cases, but maybe check if the file it references does exist.

According to our submissions rules every pull request must refer to only one addon. PRs usually have "[addon.id] version" as the PR title (although in some cases we have to rename them manually or ask the author to do so).
Sometimes there is a mistake of including more than one addon in a pull request or of producing incorrect pull requests which include older commits (for example the result of an incorrect rebase).

The CI could check if all the files included in the pull request are constrained to a single folder (with the same name as the addon id); hence detecting if the pull request is valid.

All my issues are just possible features/ideas that come into my mind for the GSOC work on the tool. I can later implement some of them if not addressed during GSOC so please do not interpret them as I am demanding work :).

Regards

Might be an edge case but check if there's an actual version bump done in addon.xml

As some devs are now putting the complete changelog in de news tag we should add a max detection with a decent character count

It isn't useful to check all addons as one has to look at the log if the reason for a failing check is caused by the changed addon or by some other addon.

We all know that our current displaying method is really not great and we need some clean way of displaying our report.
The current best option seems to have HTML reporter we can use jinja for having a nice format of the HTML page.

PROBLEM

• No records of the previous reports: Sometime we might want to see the difference of the output on two different add-ons/repositories so there wouldn't be any record of the previous run since the reporter will not append but will write to HTML file.

The following types should only have icon.png as mandatory

• Modules
• Context
• font
• language

Might be others out there?

Maybe strip the travis paths from the log outputs like
PROBLEM: Invalid xml found. /home/travis/build/xbmc/repo-plugins/plugin.video.reddit_viewer/resources/settings.xml

We should ignore hidden folders, everything starting with . like .git

See this log for some false alarms https://travis-ci.org/trakt/script.trakt/jobs/440324400

Plugin repo is most prone to error so we should add this asap :)

Currently, we use the tool for running on Travis so that addon pull requests can get validated. Any found errors get logged on Travis which developers can see and fix.

I was wondering if it would make sense to package the tool for consumption by addon developers so that they can run it directly on their machine before submitting a PR. This will help in reducing the feedback loop.

Not a lot of changes are required for this, we just need to have a cleaner CLI entry point since there are two files check_addon.py and check_repo.py and it might not be entirely obvious to other users which one needs to be run and how.

We could add a simple CLI using argparse into a file (maybe called check.py) which the users could run then we could just upload it to PyPI from where users could download this.

What do you think?

In general file handling should be refactored again and can be made more performant and elegant.

Current idea:
Walk the folder one time, create a dict and filter by file ending, then apply the checks we want on those file endings. Should work for most "one file checks".

requirements.txt doesn't seem to be shipped and thus can't be read on install.

https://travis-ci.org/Razzeee/script.trakt/jobs/439924791

Most devs are moving their addon to newer branches so we should detect if it's still available in earlier branches and detect if broken tag is added. If not it should be added.

I don't think we have a schema but we could create one.

The code in check _addon.py, check_repo.py and common.py needs to be reformatted according to PEP 8 style guide along with some improvements in code like removing unnecessary parenthesis.
Please let me know if this sounds reasonable and I can start working on a pull for this!
Regards
Aditya

xbmc/xbmc#13796

It is a general good practice to keep the entrypoints (the python files refered in addon.xml) to a minimum since they won't get compiled to bytecode. Usually the entrypoint should be something as simpler as:

import resources.lib.plugin

plugin.run()

While the main plugin code is kept in a module.
The addon-check-tool could somehow check the complexity of the entrypoint and might suggest the author to improve the addon performance by pointing to an example. A few possibilities:

1. Just check the number of uncommented lines in the entrypoint file and decide if it is lower than, lets say, 10 lines?

2. Check the number of imports on the entrypoint? This probably means the main plugin logic is executed there or, at least, there is room for improvement

3. Something more complex by exploiting cyclomatic complexity? Might be too hard to accomplish.

4. Infer from the number of python files in the addon that are not the entrypoint. This can lead to a lot of false positives though.

Option 1) seems the easiest one to implement if reforced by a few more checks