Re: contribution: git-move-heads.py

On 13 Mar 2010, at 22:12, Michael Haggerty wrote:

> 1. At the top of process_refs(), it looks like you want to clear the
> global variables tree_cache and parent_cache.  But when a variable name
> is written to within a function, it is treated as a (new) local
> variable.  If you want to rebind the global variables, then you have to
> use a "global" statement to specify that the names within process_refs()
> refer to the global variables:
> 
> def process_refs(ref_type):
>    global tree_cache, parent_cache
>    tree_cache = {}
>    parent_cache = {}
>    # ...
> 
> Or you could simply clear the dictionaries without rebinding the global
> references:
> 
> def process_refs(ref_type):
>    tree_cache.clear()
>    parent_cache.clear()
>    # ...

OK.

> 2. The move_ref() function wants a "ref_type" as its last argument, but
> in try_to_move_ref() it is called as follows:
> 
>    move_ref(ref, commit, p, ref)
> 
> Should the last argument be "ref_type"?

Doh! Fixed. Bizarrely, that didn't have a noticeable effect in my repository!

> 
> 3. Please don't use Python's conditional expression syntax "x if b else
> y" as we are still trying to retain compatibility with Python 2.4.

OK, I've changed that.

> 
> Yours,
> Michael

Thanks for the feedback. I've updated it as recommended and attached it again.



Regards,
Steve.

------------------------------------------------------
http://cvs2svn.tigris.org/ds/viewMessage.do?dsForumId=1670&dsMessageId=2459910

To unsubscribe from this discussion, e-mail: [[email protected]].