addon-check
addon-check copied to clipboard
Check if all PR files are in the same folder
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
It might make sense to only enable this via arg parameters.
So that I still can do kodi-addon-checker script.test script.test2
locally, but error on kodi-addon-checker script.test script.test2 --fail-on-multiple-addons
@enen92 For this we can check if upper directory for all the file is same or not right ?
Maybe do something like os.chdir("..")
for all the files/folders in depth 1 and then see if that directory is same or not.
@mzfr Isn't it easier to:
- get the list of changed files from the github api associated with the pull request (not familiar with it but I guess you can get them given all the commit hashes) eg:
/home/ci/addons/plugin.video.test/addon.xml
/home/ci/addons/plugin.video.test/resources/lib/plugin.py
....
- construct the base path in which the addons are stored in the repository:
addon_dir = Path('/home', 'ci', 'addons')
- compute the relative path for each file changed in the PR. eg for one of the paths:
>> relative_path = Path('/home/ci/addons/plugin.video.test/resources/lib/addon.py').relative_to(addon_dir)
>> print(relative_path)
<< PosixPath('plugin.video.test/resources/lib/addon.py')
- Grab the basedir:
>> relative_path.parts[0]
<< 'plugin.video.test'
- Check if the result for all files is the same. If not the Pull request is incorrect.
@enen92 sorry thats what I meant by "upper directory". But you explained it in detail. Thank you
I'm not sure about thise, especially if we need to call the github api. It means we add another hard dependency and very closely couple this checker with our repo rules on prs. Not with good rules for coding.
So if we would run this locally with vscode some day it will just tilt untill we do some weird setup. Maybe it's a different concern and would be better of in it's own tool?
add the following to .travis.yml
- 'if [ "$TRAVIS_PULL_REQUEST" != "false" ]; then if [ "$(git diff --diff-filter=d --name-only HEAD~ | grep / | cut -d / -f1 | sort | uniq | wc -l)" -gt "1" ]; then exit 1; fi fi'
@Razzeee @enen92 This is done right ?
No, I haven't updated our travis scripts yet
If you can tell me for what all branches travis script are to need to be updated then I can do it :slightly_smiling_face:
Well we need to add the stuff posted by rechi above and maybe also add the needed travis-buddy command, that I already added as a test on the plugins repo for the krypton branch.
@Razzeee any updates on this one ?
@Rechi is that code snippet still how it should look?
Yes, but it would mark some of our own PRs in repo-resources as failed (e.g. mass updates to game.controler.*).
So we might want to not do this I guess.
I think we still need this, mass updates to addons are not common and are usually handled only by team members. In most situations 1 PR = 1 addon update. Would it be possible to add a label to a pull request and make travis ignore the check? Something like the "No-Jenkins" label in Jenkins?
Here is the script which just checks every commit of a PR separately.
if [ "$TRAVIS_PULL_REQUEST" != "false" ]; then
git rev-list --no-merges --reverse HEAD~..HEAD | while read rev; do
if [ "$(git diff --diff-filter=d --name-only $rev~..$rev | grep / | cut -d / -f1 | sort | uniq | wc -l)" -gt "1" ]; then
echo 'The following commit changes more then one add-on'
git --no-pager show --stat $rev
exit 1
fi
done
fi
@Rechi This script is all we need to add in the .travis.yml
file right? or is there any other changes required to fix the issue?