-
Notifications
You must be signed in to change notification settings - Fork 44.6k
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
Allow interrupting continuous iteration #2291
Conversation
This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request. |
@ehei hey sorry for the recent conflicts, if you push we'll definitely prioritize this. |
Nice PR! We can't merge into |
Ah, sorry, I'll switch it over. Looked like stable was the new
main/master.
…On Tue, Apr 18, 2023, 11:33 AM Reinier van der Leer < ***@***.***> wrote:
We can't merge into stable though, so please change the base branch to
master.
—
Reply to this email directly, view it on GitHub
<#2291 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAVTCVC5ILMHUDJGAOZDSTTXB2X3JANCNFSM6AAAAAAXB7ACVU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@ehei can you reopen when ready for review/merge? |
Overlaps with #2069 |
Yeah I saw that PR - not a fan of having to do multiple button mashing, but
if that PR is ready to go, I can come back around and remake the
tests/refactoring I was doing in my PR after that one is merged in,
reevaluating around the other PR.
If it's not ready, I'll get to work on getting this ready to actually put
in.
Let me know.
…On Tue, Apr 18, 2023 at 4:27 PM Reinier van der Leer < ***@***.***> wrote:
Overlaps with #2069
<#2069>
—
Reply to this email directly, view it on GitHub
<#2291 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAVTCVDY6YWFCW5HQJA6HKTXB32JRANCNFSM6AAAAAAXB7ACVU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@ehei please reopen and @BillSchumacher will fix it |
Blocked by #2481 |
Background
While running with the 'y -XXX' prompt, I found myself wanting to know how many steps had been performed, and how many were left out of how many I had authorized. Additionally, I wanted a way to interrupt the process without resorting to crashing the system or killing a process outright.
Changes
I created several variables to track both if an interrupt of the process was desired, by checking for the CTRL key being held down, and provided feedback in the output to show how many steps have been completed out of the total, and what the percentage done was.
Documentation
I add several comments like at line 472 (check for CTRL press, interrupt) and a comment where the addition to the prompt for feedback was created, as well as where the percentage done was calculated. Atomic tests for those could be pulled out, but I didn't want to continue falling being all the development. Happy to follow this up with pulling out methods and/or developing some decent tests if warranted.
Test Plan
Ran the app with several scenarios, including:
PR Quality Checklist
As stated earlier, I did not add any tests for this PR, but would be happy to revisit tests and refactoring the user prompting and feedback if desired.
Looking forward to any and all feedback. Thanks!