fab-user
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Fab-user] Patch for prompt and require


From: Christian Vest Hansen
Subject: Re: [Fab-user] Patch for prompt and require
Date: Mon, 20 Oct 2008 18:26:04 +0200

On Mon, Oct 20, 2008 at 5:05 PM, Niklas Lindström <address@hidden> wrote:
> Hi Christian!
>
> I've addressed some things in my new branch (fork..) which I've cloned
> from github, but I'm afraid I still feel somewhat uncomfortable with
> git. :(
>
> I'm not sure whether to work on my own master branch, nor if I should
> create a branch in my repo in which to pull in your public master..
> Right now I've added changes to master in my fork, and then
> fetched+merged the change to markdown docstrings (using git remote to
> manually track your repo). This has been pushed to my github repo.

I'm not that experienced in working with repos that are forks, but I
put stuff in my master branch that I intend to push at some point and
I create local branches for everything else.

When I integrated parts of your patch, I created a new local branch
and, while perusing the diff, selectively applied the parts I liked.
Then I fiddled the bytes a little until I was satisfied and merged it
into my master for pushing.

That process won't be much different with 3'rd party remote branches.

>
>
> Here's what I've continued with:
>
> On Sun, Oct 19, 2008 at 4:18 AM, Christian Vest Hansen
> <address@hidden> wrote:
>> I've accepted your changes to require() but I'm not sure about the
>> prompt() changes. I think prompt is getting a little complex/hard to
>> understand. Also, it'd be nice if you had updated the doc-string.
>
> Great; I'm glad that you found the changed require() useful.
>
>> Although, I'm not sure what to do instead.
>
> I understand your concerns with what I did with prompt(). I'm after
> three things:
>
> 1. To (always) repeat the prompt if validate fails. I skipped the
> 'repeat' flag in my new version.
>
> 2. To invoke a given validate even if we have a supplied value. I just
> removed my 'auto' keyword and changed the use of a present varname in
> ENV so that this gets validated as well..
>
> 3. I'd like to use regexp validations, so I've kept the overloaded
> 'validate' (and updated the docstring) which uses the RegexpValidator
> (previously named Matcher). Do you think allowing regexp strings is a
> bad idea? Perhaps we could keep RegexpValidator or similar, so at
> least that one is available as a convenience tool? Though think I'd
> prefer a less "obtrusive" helper..

These three points make It _sound_ better.

I don't know if Jeff has put any code towards any of his ideas
regarding prompt, though I don't see anything in the commit log for
his master branch.

>
>
>>> (I haven't used the git-mechanism for mailing patches, just
>>> generation. If I'm going to continue like this, perhaps I should also
>>> set up my branch on git-hub, as Jeff has?)
>>
>> A github branch is a good idea. Cherry-picking, merging and viewing
>> changes online is easy, and the commit message and author is
>> preserved.
>
> Working on doing this properly. :)
>
>
>>> == To do next ==
>>>
>>> * add unittests for the above (I've only "acceptance"-tested the above
>>> in my actual fabfiles).
>>
>> Ah, yes. The unit testing story is a little weak. I'm afraid I've been
>> sloppe in that department :(
>
> I'll see if I can add a suite of all commands. Personally I really
> like Nose (<http://pypi.python.org/pypi/nose>) for driving tests. Do
> you have any preferences in this department?

I don't have any preferences. Not even for the feeble attempt already
in the code base.

>
>
>>> * improved "load":
>>>    - will only invoke the loaded fab script once. Thus multi-file based fab
>>>      setups can shared loaded scipts without having them invoked multiple
>>>      times.
>>>
>>> * new operation "call_once":
>>>    - will call the given function (command) *unless* it has already been
>>>      called.
>>
>> These ideas I like. Especially the improvements to load.
>
> I have *preliminary* working versions of these. Especially call_once
> needs more thinking though (regarding messages, and if we could reuse
> the '_execute_commands' logic for parameters etc -- see my TODO
> scribble).

You can use pull-request (github feature) or send an email when you
think it's ready.

>
>
>>> * new decorators
>>>
>>>    @requires
>>>       Calls "require" with the given args before invoking the decorated
>>>       command.
>>>
>>>    @depends
>>>        Calls "call_once" with the given args before calling the decorated
>>>        command.
>>
>> These would be the first decorators intended for use in fabfiles.
>> You/we will have to think about how they are going to interoperate
>> with the list and help commands.
>
> Absolutely. I've added these, without any documentation, and will
> continue with how to register them and integrating that in the help
> system. '@depends' will also be aligned with how I'll proceed with
> improving 'call_once'.

Sounds good :)

>
>
>>> == After that ==
>>>
>>> * replace current "set" and "get" with a more powerful "params" object.
>>
>> They're called variables in fabric terminology. I wonder what you have
>> in mind here.
>
> Basically I just like to (as Jeff also seemed to) solve the issue that
> 'set' currently shadows the builtin with the same name. We then toyed
> with the idea of exposing one object to replace the get/set (which I
> fully understand if you don't want to remove though for e.g.
> compatability, clean names etc). Since 'vars' is also a builtin, I
> thought of 'params'. Perhaps e.g. 'var' would be better.. Working
> something like:
>
>    # Define one or more variables (just as with current "set"):
>    var(fab_user='me')
>    var(fab_hosts=[...], fab_user='me')
>
>    # Define one var can also be done like:
>    var.fab_user = 'me'
>    var['fab_user'] = 'me'
>
>    # For getting them, same pattern:
>    print var.fab_user
>    print var['fab_user']

Looks nice. I think `var` is a good name. I wasn't totally happy with
the shadowing when I introduced `set` but I couldn't come up with a
better name.

It's OK to break a little backwards compatibility when the version
series changes from 0.0.x to 0.1.x.

>
> ----
>
> Unless I'm working backwards, I hope my use of git and github will be
> of help for you in tracking my suggestions. Please ask me to fix
> documentation and complete/reimplement what I have working now, in
> case you'll eventually like to merge more of it.
>
> Best regards,
> Niklas
>



-- 
Venlig hilsen / Kind regards,
Christian Vest Hansen.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]