[PATCH 0 of 2] Progress indicator and support for pull and update

Stefano Tortarolo stefano.tortarolo at gmail.com
Wed Jan 7 11:37:40 CST 2009


2009/1/6 Dirkjan Ochtman <dirkjan at ochtman.nl>:
> Stefano Tortarolo <stefano.tortarolo <at> gmail.com> writes:
>> Quick update... I've pushed my patches here:
>> http://www.bitbucket.org/astratto/progress
>
> I just reviewed the most recent version of Stefano's core patch.

Thank you very much.
(Sorry Dirkjan for the double-post, I forgot to hit "reply all")

> I have some concerns that the current API is more complex than it needs to be.
> Specifically, there are many getters and setters, which in my opinion should be
> replaced by simple attributes or property()s.

Yes, you're right. I'm used to using getters/setters due to my java
background... I'll replace them as soon as possible.

> I also think the ui.progress() call that creates a progresshandler should just be folded into the
> progress.createprogress() function.

If I understand you correctly you mean to move everything out of ui.py?
This way we won't have to do ui.progress i.e. in merge.py but
something like progress.createprogress()?
Honestly I'd do the opposite, folding progress.createprogress() into
ui.progress() since ui has to be aware of progress.

> Additionally, the output abstraction currently happens by splitting off a
> 'progresshandler' from a 'progressdisplayer', but I have a feeling that this
> could well be simpler, for example by collapsing them into a single class
> (allowing output to be overridden by using a subclass or mixin).

Actually that's the opposite direction the patch took since it's creation.
Anyway I'll post another collapsed version to compare them ok?

Thanks again for reviewing that patch and for your comments.

Cheers,
Stefano


More information about the Mercurial-devel mailing list