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

processing: extract progress reporting from business logic #680

Merged
merged 1 commit into from
Nov 15, 2023

Conversation

vlaci
Copy link
Contributor

@vlaci vlaci commented Nov 14, 2023

While trying to update pyright, it started complaining about potentially unbound variables1. These are not actual correctness issues, but signals some code smell due to conditionally configured variables.

The progress bar handling got extracted from _process_task, and got converted to a context manager, so its lifetime management became simpler. To eliminate if conditions sprinkled everywhere, a NullProgressDisplay is also added for the case where we want to do nothing.

Footnotes

  1. /home/vlaci/devel/git/github.com/onekey-sec/unblob/unblob/processing.py
      /home/vlaci/devel/git/github.com/onekey-sec/unblob/unblob/processing.py:163:36 - error: "progress_display" is possibly unbound (reportUnboundVariable)
      /home/vlaci/devel/git/github.com/onekey-sec/unblob/unblob/processing.py:164:13 - error: "progress_display" is possibly unbound (reportUnboundVariable)
      /home/vlaci/devel/git/github.com/onekey-sec/unblob/unblob/processing.py:165:17 - error: "overall_progress_task" is possibly unbound (reportUnboundVariable)
      /home/vlaci/devel/git/github.com/onekey-sec/unblob/unblob/processing.py:167:23 - error: "progress_display" is possibly unbound (reportUnboundVariable)
      /home/vlaci/devel/git/github.com/onekey-sec/unblob/unblob/processing.py:184:9 - error: "progress_display" is possibly unbound (reportUnboundVariable)
      /home/vlaci/devel/git/github.com/onekey-sec/unblob/unblob/processing.py:184:38 - error: "overall_progress_task" is possibly unbound (reportUnboundVariable)
      /home/vlaci/devel/git/github.com/onekey-sec/unblob/unblob/processing.py:185:9 - error: "progress_display" is possibly unbound (reportUnboundVariable)
    7 errors, 0 warnings, 0 informations
    

@vlaci vlaci force-pushed the refactor-progress-reporting branch 2 times, most recently from 72fae3a to 00982e0 Compare November 14, 2023 16:30
While trying to update pyright, it started complaining about
potentially unbound variables[^1]. These are not actual correctness
issues, but signals some code smell due to conditionally configured
variables.

The progress bar handling got extracted from `_process_task`, and got
converted to a context manager, so its lifetime management became
simpler. To eliminate if conditions sprinkled everywhere, a
`NullProgressDisplay` is also added for the case where we want to do
nothing.

[^1]:

    ```
    /home/vlaci/devel/git/github.com/onekey-sec/unblob/unblob/processing.py
      /home/vlaci/devel/git/github.com/onekey-sec/unblob/unblob/processing.py:163:36 - error: "progress_display" is possibly unbound (reportUnboundVariable)
      /home/vlaci/devel/git/github.com/onekey-sec/unblob/unblob/processing.py:164:13 - error: "progress_display" is possibly unbound (reportUnboundVariable)
      /home/vlaci/devel/git/github.com/onekey-sec/unblob/unblob/processing.py:165:17 - error: "overall_progress_task" is possibly unbound (reportUnboundVariable)
      /home/vlaci/devel/git/github.com/onekey-sec/unblob/unblob/processing.py:167:23 - error: "progress_display" is possibly unbound (reportUnboundVariable)
      /home/vlaci/devel/git/github.com/onekey-sec/unblob/unblob/processing.py:184:9 - error: "progress_display" is possibly unbound (reportUnboundVariable)
      /home/vlaci/devel/git/github.com/onekey-sec/unblob/unblob/processing.py:184:38 - error: "overall_progress_task" is possibly unbound (reportUnboundVariable)
      /home/vlaci/devel/git/github.com/onekey-sec/unblob/unblob/processing.py:185:9 - error: "progress_display" is possibly unbound (reportUnboundVariable)
    7 errors, 0 warnings, 0 informations
    ```
@vlaci vlaci force-pushed the refactor-progress-reporting branch from 00982e0 to 9d940c8 Compare November 14, 2023 16:52
@e3krisztian
Copy link
Contributor

Nice! :)

@e3krisztian e3krisztian requested a review from qkaiser November 14, 2023 17:25
@qkaiser qkaiser merged commit 6cf2950 into main Nov 15, 2023
11 of 12 checks passed
@qkaiser qkaiser deleted the refactor-progress-reporting branch November 15, 2023 10:26
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.

3 participants