Re: [Fab-user] Code review for changes in 1.x

[Michael Gliwinski]

On Sunday 13 Feb 2011 16:50:10 Travis Swicegood wrote:
> As many of you may know, I've long had a fork of Fabric that adds some
> goodies in such as @task decorators and module level tasks (i.e., fab
> prod.deploy) and a few other things.  I've created a Pull Request on GitHub

Good job!  There are some nice things in there that I'd be happy to see merged 
into Fabric indeed.

On a general note though, wouldn't it be easier (for reviewers and for 
merging) if these things were separated into feature branches?  The different 
features there don't seem to depend on each other anyways.

> Any thoughts you have would be appreciated.  I don't think this will be the
> final solution for @task style task declaration, and I know it isn't the
> final Task object.  There's still a lot to add/refactor before this is
> through.  The changes in this pull request are meant as a first step, and
> as such are entirely backwards compatible.

Regarding overall approach, what made you choose the Task class as a way to 
designate tasks?  I'm asking because it doesn't appear to do anything special 
and has a disadvantage of replacing the original callable and changing its 
signature.

I'm just thinking aloud here but it seems to me a registry approach may be 
simplier.  Fabric already has a sort of task registry (the tasks dict used in 
load_fabfile) so it would be a matter of making it global and changing how 
it's populated.

By registry here I mean either a simple dict as is already used or something 
more fancy like e.g. Registry implementation in bzrlib [1] which allows 
storing both objects (or indirect references) and metadata.

I like the namespacing implementation, although because "explicit is better 
than implicit" I agree with Jeff's comment to #56 [2] about having a 
register_subtasks or similar function.


  [1]: http://bazaar.launchpad.net/~bzr-
pqm/bzr/bzr.dev/view/head:/bzrlib/registry.py#L74
  [2]: http://code.fabfile.org/issues/show/56#note-3


-- 
Michael Gliwinski
Henderson Group Information Services
9-11 Hightown Avenue, Newtownabby, BT36 4RT
Phone: 028 9034 3319

**********************************************************************************************
The information in this email is confidential and may be legally privileged.  
It is intended solely for the addressee and access to the email by anyone else 
is unauthorised.
If you are not the intended recipient, any disclosure, copying, distribution or 
any action taken or omitted to be taken in reliance on it, is prohibited and 
may be unlawful.
When addressed to our clients, any opinions or advice contained in this e-mail 
are subject to the terms and conditions expressed  in the governing client 
engagement leter or contract.
If you have received this email in error please notify address@hidden

John Henderson (Holdings) Ltd
Registered office: 9 Hightown Avenue, Mallusk, County Antrim, Northern Ireland, 
BT36 4RT.
Registered in Northern Ireland
Registration Number NI010588
Vat No.: 814 6399 12
*********************************************************************************