Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't process hidden folders #123

Open
razzeee opened this issue Oct 11, 2018 · 12 comments · Fixed by #143
Open

Don't process hidden folders #123

razzeee opened this issue Oct 11, 2018 · 12 comments · Fixed by #143

Comments

@razzeee
Copy link
Member

razzeee commented Oct 11, 2018

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

@ghost
Copy link

ghost commented Oct 11, 2018

It seems to me that there is already something that does it.

if addon_folder[0] != '.':

When do you want to skip hidden folders ?

@razzeee
Copy link
Member Author

razzeee commented Oct 12, 2018

the log you see above is not using the check_repo function as it's not being run on a repo. I think it uses the other way to be run, which is via the check_addon module https://github.com/xbmc/addon-check/blob/master/kodi_addon_checker/__main__.py#L47

I just realized, that we only want this on a single addon run. If we're running for a repo, it should still report hidden folders, as those should not be included.

@ghost
Copy link

ghost commented Oct 12, 2018

Okay fine I had trouble reading the logfile.

Edit: you’re right since addon.xml is present at the root of
https://github.com/trakt/script.trakt

@ghost
Copy link

ghost commented Oct 14, 2018

https://github.com/xbmc/addon-check/blob/master/kodi_addon_checker/handle_files.py#L33-L44

@Razzee so just a new conditional in this for loop and then appropriate tests right ?

I noticed that os.walk also returns its argument as the first element. Is it okay for the whole addon to be a hidden directory ? If so I should skip the condition on the first iteration. And if not, an error should be thrown; otherwise file_index will remain empty.

@razzeee
Copy link
Member Author

razzeee commented Oct 14, 2018

It's not okay for an addon to be in a hidden dir.

The method you pointed out seems to be the right spot for this change. I don't think you need to throw any error, just ignore the folder.

@ghost
Copy link

ghost commented Oct 14, 2018

If it’s not okay, better test for it and add an entry to the report right ?

@razzeee
Copy link
Member Author

razzeee commented Oct 14, 2018 via email

@ghost
Copy link

ghost commented Oct 14, 2018

Ok I’ll PR something on Monday.

@ghost
Copy link

ghost commented Oct 15, 2018

Oh I’m sorry @mzfr I didn’t see your pr

@LordGameleo
Copy link
Contributor

Is this issue open?? @razzeee

@razzeee
Copy link
Member Author

razzeee commented Mar 3, 2019

It's not solved, but I think it might not be needed anymore

@LordGameleo
Copy link
Contributor

Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants