[MERGE REVIEW] create_checkout_convenience

[Aaron Bentley]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin Pool wrote:
> When talking to Robert the other day I proposed this rule: static/class
> methods should be confined to the "named constructor" pattern.

I'll have to get back to you about whether this should be a general
rule.  It seems strange to make things instance methods when they don't
require access to instance variables.

> Could these perhaps instead be, say, an instance method of source?  You
> must already have one before starting, and it seems reasonable to say
> "please check yourself out".o

That sounds okay to me.

> And also the 'lightweight' parameter basically runs two different
> methods, which suggests we should actually have two.  So how abut
> 
>   source.create_lightweight_checkout(to_location, ...)

The reasoning behind that was that we're trying to present lightweight
checkouts and heavyweight ones as behaving essentially the same.  At
this level, I think if we make them separate methods, we make the client
work harder, for no great gain.  e.g. the implementation of cmd_checkout
gets more complicated:

- - source.create_checkout_convenience(to_location

- -                                    revision_id, lightweight)
+ if lightweight is True:
+    source.create_lightweight_checkout(to_location, revision_id)
+ else:
+    source.create_heavyweight_checkout(to_location, revision_id)

cbranch would similarly be more complicated.

I'm not stuck on it, it just seems like our convenience methods should
tend to resemble our commands.  Otherwise, people will prefer to invoke
the commands rather than the convenience methods.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.2 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFE1tYf0F+nu1YWqI0RAkD9AJ0cqCLKzufvwIaEONEVRWbUkg8N4ACeIVBE
0SuM8HHP5Zs6VfgDfnhewnY=
=l+1c
-----END PGP SIGNATURE-----