-
Notifications
You must be signed in to change notification settings - Fork 3
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
Refactor the interface to progress bar updates #235
Comments
My proposalI think what humans expect here is (just illustrating idea, this might eventually look differently): data ProgressItemStatus = Unchecked | Ok | Fixable | Failed
(->) :: ProgressItemStatus -> ProgressItemStatus -> Progress a -> Progress a -- example
let progress' = progress & Unchecked -> Ok This way, intention behind some of the updates may be expressed more clearly: progressUpdateOnRetry =
(if retryCounter == 0 then Unchecked else Fixable)
->
(if retryCounter < maxRetries then Fixable else Failed) |
Another, more complex but better proposalWith both the current solution and the improvement I suggested above there is an issue - we have to figure out the old status correctly for things to work. And this is pretty error-prone, not saying that it may complicate future features (e.g. if we decide to track redirects too). So instead we could internally keep |
Problem: the current progress bar interface is quite low-level and error-prone. Solution: the proposed solution here modifies both the progress bar internal model and interface by representing process units by witnesses. It does not require great changes, but the resulting interface is more clear and less error-prone.
Problem: the current progress bar interface is quite low-level and error-prone. Solution: the proposed solution here modifies both the progress bar internal model and interface by representing progress units by witnesses It does not require great changes, but the resulting interface is more clear and less error-prone.
Regarding the second proposal, it looks really natural, but creating unique tokens for each verification item can require great changes, because I guess that these would be created when initialising the progress bar and then each verification item should keep track of its own token.
Currently, we count unique links to calculate the total amount of work to do, and then the verification process implements a request/response cache for external resources, so it seems that duplicated verification of them is avoided. I have proposed another solution in #265, inspired by that one, which I think that is simple, does not require great changes and also makes the progress-bar interface less low-level and error prone. |
Problem: The current progress bar interface is quite low-level and error-prone. Solution: The proposed solution here modifies both the progress bar internal model and interface by representing progress units by witnesses It does not require great changes, but the resulting interface is more clear and less error-prone.
Problem: The current progress bar interface is quite low-level and error-prone. Solution: The proposed solution here modifies both the progress bar internal model and interface by representing progress units by witnesses. It does not require great changes, but the resulting interface is more clear and less error-prone.
…-interface [#235] Refactor progress bar interface
Clarification and motivation
For providing a progress bar interface we provide functions:
incProgress
,incProgressUnfixableErrors
,decProgressFixableErrors
, but they constitute quite a low-level interface. If I useincProgressFixableErrors
, it should be accompanied byincProgress
, forgetting it is likely an error.fixableToUnfixable
is better in this regard.Let's discuss in comments how to better improve this situation.
Acceptance criteria
Progress
constructor to make a datatype) is updated to use this new interface.The text was updated successfully, but these errors were encountered: