-
Notifications
You must be signed in to change notification settings - Fork 498
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
Show envs when run2 #802
Show envs when run2 #802
Conversation
I think that makes sense, thank you @mahomahomaho |
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.
Okay, I just made changes. However, could someone test them, because I stopped using buildozer (in favour of pure p4a) so I cannot check them.
Thanks for taking another look. I just realised that yes that hardcoded number things was already in our code base 😅 I'll unit test some of it, refactor and probably merge your PR after further review |
I've created #831 to help with testing the logger before we can merge yours |
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.
Thanks!
@@ -282,6 +296,7 @@ def cmd(self, command, **kwargs): | |||
else: | |||
self.debug('Run {0!r} ...'.format(command.split()[0])) | |||
self.debug('Cwd {}'.format(kwargs.get('cwd'))) | |||
self.log_env(self.DEBUG, kwargs["env"]) |
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.
I think that like should be removed.
I'm seeing it only now after using buildozer master for a while.
For instance during the auto accept license process, this log_env
seems to be printing every time. We want it only after crashes, not for every command call
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.
I've created a PR to remove that line #843
When some command fails, it's good to know what envs was set, to be able to reproduce error.
Also few formatting changes forced by flake8