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

Improve: Remove some clutter from debug.log #635

Merged
merged 4 commits into from
May 2, 2017

Conversation

stoivo
Copy link
Member

@stoivo stoivo commented Apr 17, 2017

It's not super nice but I prefer to have setting as arguments instead of state on an object.

@@ -72,7 +72,8 @@ def git(self, *args,
decode=True,
encode=True,
stdin_encoding="UTF-8",
custom_environ=None):
custom_environ=None,
skip_adding_to_log=False):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems to me that it overkills. Maybe something like this (untested)?

from contextlib import contextmanager
@contextmanager
def disable_logging():
    util.debug.stop_logging()
    try:
        yield
    finally:
        util.debug.start_logging()


with disable_logging():
    short_status = self.get_branch_status_short()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

start_logging() is resetting the log.
but it looks better

@stoivo
Copy link
Member Author

stoivo commented Apr 28, 2017

@randy3k, I did't know of contextmanager. This is so much cleaner 🎉

@@ -57,7 +58,8 @@ def run_async(self):
self.view.erase_status("gitsavvy-repo-status")
return

short_status = self.get_branch_status_short()
with disable_logging():
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

debug.disable_logging?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are so right.

@randy3k randy3k merged commit 2938321 into timbrel:master May 2, 2017
@stoivo stoivo deleted the clean_log branch May 2, 2017 21:18
@stoivo
Copy link
Member Author

stoivo commented May 2, 2017

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants