[merge] push and pull to use Result objects

Martin Pool mbp at sourcefrog.net
Thu Mar 1 02:02:59 GMT 2007


John Arbash Meinel wrote:
> John Arbash Meinel has voted +1 (conditional).
> Status is now: Semi-approved
> Comment:
> I like having more information returned by push/pull.
> 
> I wonder if we want to define __str__ rather than just __int__. I
> thought __int__ only came into play if someone did int(X). And I don't
> think people are doing:
>  count = Branch.pull()
>  int(count)

No, but they are doing

  if result > 0

and

  "%d revisions pulled"

I thought about doing a str, but it wasn't clear to me if it should give
str(int(self))

> I don't really like that the Result objects do not have members
> pre-defined. It means that if you want to figure out what you are
> supposed to look for, you have to go to the code that uses them, and
> figure it out. Or do introspection of the objects in an interpreter.
> 
> I'm guessing it was because you didn't really know what you wanted to
> add. But I think you have figured it out by now.

I'll document what they're supposed to have; they should always be set.

> 
> If you want it to be easy to add new key=>value pairs (which is really
> all members are) I would rather see it as an explicit dictionary. So
> that people see it as such, rather than expecting them to use
> result.__dict__ or something like that.

But harder to do things like the __int__ trick, and I think callers find
it easier to use dot rather than indexing.

> 
> This isn't strictly backwards compatible, but I think it is a simple
> enough upgrade that it is worth breaking. I do think it would be good
> for the 'bzr pull' output to spit out more than the current revno. I
> find "bzr update" to be pretty confusing because it doesn't give me any
> sense of context. I would prefer at least "Now at revno 2 (from revno
> 1)". Or something like that.

OK, so combined with aaron's feedback i think i will add the docs and
put it in.

-- 
Martin



More information about the bazaar mailing list