-
Notifications
You must be signed in to change notification settings - Fork 496
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
Migrate all usages of mkdir and rmdir to buildops #1668
Conversation
This is a refactor as one step to reduce the size/complexity of the Buildozer class. It doesn't change functionality. except to improve logging consistency and a small performance improvement * mkdir and rmdir removed from `buildozer/__init__.py` * All references to buildozer's mkdir and rmdir changed over to use buildops. * Call to Linux's mkdir via a separate shell replaced with (faster, and platform independent) library call. * Re-ordered imports, where touched, to match PEP8.
buildozer/__init__.py
Outdated
@@ -361,22 +362,22 @@ def check_build_layout(self): | |||
exit(1) | |||
|
|||
# create global dir | |||
self.mkdir(self.global_buildozer_dir) | |||
self.mkdir(self.global_cache_dir) | |||
mkdir(self.global_buildozer_dir) |
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.
To help people understand that mkdir
(and other methods) are from buildops
, how about importing buildops
and than using buildops.mkdir
?
Yeah, is quite verbose, but IMHO, the readability (and the understandability) gets improved a lot.
What do you think about it?
I don't mind either way. Will try to get an updated version to you tomorrow.
…On Fri, 25 Aug 2023, 00:52 Mirko Galimberti, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In buildozer/__init__.py
<#1668 (comment)>:
> @@ -361,22 +362,22 @@ def check_build_layout(self):
exit(1)
# create global dir
- self.mkdir(self.global_buildozer_dir)
- self.mkdir(self.global_cache_dir)
+ mkdir(self.global_buildozer_dir)
To help people understand that mkdir (and other methods) are from buildops,
how about importing buildops and than using buildops.mkdir?
Yeah, is quite verbose, but IMHO, the readability (and the
understandability) gets improved a lot.
What do you think about it?
—
Reply to this email directly, view it on GitHub
<#1668 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAHG6NJLVRSE3EKV5K76NODXW5TDFANCNFSM6AAAAAA33EM3UM>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
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!
* Migrate all usages of mkdir and rmdir to buildops This is a refactor as one step to reduce the size/complexity of the Buildozer class. It doesn't change functionality. except to improve logging consistency and a small performance improvement * mkdir and rmdir removed from `buildozer/__init__.py` * All references to buildozer's mkdir and rmdir changed over to use buildops. * Call to Linux's mkdir via a separate shell replaced with (faster, and platform independent) library call. * Re-ordered imports, where touched, to match PEP8. * Make buildops methods more explicit
This is a refactor as one step to reduce the size/complexity of the Buildozer class. It doesn't change functionality. except to improve logging consistency and a small performance improvement
buildozer/__init__.py
(cleaner versions are already inbuildops.py
)