Code review comment for lp:~agateau/checkbox/progress-bar

Daniel Manrique (roadmr) wrote :

Hi Aurélien!

Thanks for updating the glade file, I hope you're OK with the changes and it still fits your purpose.

One last thing before we move forward. I notice you're still carrying the progress information in the job itself. As I mentioned before, this feels like a forced use of the job to shuttle information to the frontend. I'm wondering if you have a specific reason for preferring this implementation; maybe it's something you are using elsewhere, or have future plans for it.

In any case, the mechanism I suggested (where an event gets fired which then is caught by the user_interface plugin which in turn sets a value in the UserInterface itself) is an alternative, although it's more complicated it avoids using the job for this purpose.

Please let me know if this is something you definitely want to be as you propose, or you'd be open to considering the other alternative (the progress bar will still work the same way).

Thanks!

review: Needs Information

« Back to merge proposal