-
Notifications
You must be signed in to change notification settings - Fork 503
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
Factor out logging code from Buildozer class #1632
Conversation
buildozer/targets/osx.py
Outdated
@@ -20,14 +22,14 @@ def ensure_sdk(self): | |||
join(self.buildozer.platform_dir, 'kivy-sdk-packager-master')): | |||
self.buildozer.info( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed a few self.buildozer.info
s.
Result of self review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Julian-O !
In buildozer/scripts/remote.py
references have not been updated.
Can you please take care of it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems we still have two old references:
buildozer/buildozer/scripts/client.py
Line 19 in 0851a10
Buildozer().error('%s' % error) |
buildozer/buildozer/scripts/remote.py
Line 274 in 0851a10
Buildozer().error('%s' % error) |
Unfortunately I missed them from the previous review.
(That part will likely need a huge cleanup, but makes sense to keep it updated ATM)
These sneaky ones created a Buildozer() object just to call logging! I missed them in my text searches. That left me red-faced. I should have caught these, sorry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you!
(The CI pipeline is failing, but is a known issue with Cython 3.0.0)
First step toward #1629.
Code related to logging moved into its own class/file.
References (including in targets and tests) updated.
Unit tests separated out, and enhanced to test more conditions.
Trivial indenting/grammar issues in osx.py fixed.
No-op exception handling code removed.