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

Search for print statements and report them #19

Open
razzeee opened this issue Dec 14, 2017 · 10 comments · May be fixed by #150
Open

Search for print statements and report them #19

razzeee opened this issue Dec 14, 2017 · 10 comments · May be fixed by #150

Comments

@razzeee
Copy link
Member

razzeee commented Dec 14, 2017

No description provided.

@Rechi
Copy link
Member

Rechi commented Dec 14, 2017

@razzeee are you already working on this?

@razzeee
Copy link
Member Author

razzeee commented Dec 14, 2017 via email

@MartijnKaijser
Copy link
Member

Remember that some use print statements for unit tests

@mzfr
Copy link
Contributor

mzfr commented Feb 16, 2018

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

@razzeee
Copy link
Member Author

razzeee commented Feb 19, 2018

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.

@Rechi
Copy link
Member

Rechi commented Feb 20, 2018

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.

@mzfr
Copy link
Contributor

mzfr commented Aug 21, 2018

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?

@razzeee
Copy link
Member Author

razzeee commented Aug 21, 2018

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.

@romanvm
Copy link

romanvm commented Oct 14, 2018

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.

@MartijnKaijser
Copy link
Member

unit testing outside kodi

@mzfr mzfr linked a pull request Dec 10, 2018 that will close this issue
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.

5 participants