addon-check icon indicating copy to clipboard operation
addon-check copied to clipboard

Search for print statements and report them

Open razzeee opened this issue 7 years ago • 10 comments

razzeee avatar Dec 14 '17 09:12 razzeee

@Razzeee are you already working on this?

Rechi avatar Dec 14 '17 16:12 Rechi

No, just came to the conclusion seeing the other pr. Basically there should be everything in place, we just need to change the regex it's looking for.

On Thu, 14 Dec 2017, 17:13 Rechi, [email protected] wrote:

@Razzeee https://github.com/razzeee are you already working on this?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/xbmc/addon-check/issues/19#issuecomment-351757202, or mute the thread https://github.com/notifications/unsubscribe-auth/AFqyZL6wD1S7ujHSMPUmtWWr5_08eStwks5tAUkMgaJpZM4RBymi .

razzeee avatar Dec 14 '17 16:12 razzeee

Remember that some use print statements for unit tests

MartijnKaijser avatar Dec 14 '17 19:12 MartijnKaijser

So, should we just grep for print statements in the code and report something like "hey, are you sure this print is intended"?

And by grep I actually mean something python-code-aware like the python's ast module

mzfr avatar Feb 16 '18 09:02 mzfr

Code aware would be awesome, but I'm not sure how it will handle third party deps like our kodi classes, which are nowhere to be found here :) And yes, only prints would probably be the best. But then again, that would mean they don't show up in PRs and every reviewer must check the travis log and then check each of the files for false/positives. Doesn't feel like we are on a good path here.

razzeee avatar Feb 19 '18 22:02 razzeee

We could classify using print statements only as warnings. Additionally run 2to3 to check if print at least is in the new python3 syntax and only treat that case as fatal error if it isn't.

Rechi avatar Feb 20 '18 07:02 Rechi

so basically we want to search for print statement in all the addon files(.py) and if it's there then they should be reported right?

mzfr avatar Aug 21 '18 14:08 mzfr

Yes, as a warning. But we're not sure how useful a check like this will be in reality. So it might just turn out to be useless. But we need to try.

razzeee avatar Aug 21 '18 19:08 razzeee

Technically, in Kodi ``print` is is implemented is implemented as xbmc.log(message, DEBUG): https://github.com/xbmc/xbmc/blob/master/xbmc/interfaces/python/AddonPythonInvoker.cpp#L19

So I'm not sure if we need to bother with print statements in addons.

romanvm avatar Oct 14 '18 14:10 romanvm

unit testing outside kodi

MartijnKaijser avatar Oct 14 '18 14:10 MartijnKaijser