[MERGE][trivial] builtins._create_prefix use 'location', give it a parameter, thank you
Vincent Ladeuil
v.ladeuil+lp at free.fr
Thu Jun 21 07:14:10 BST 2007
>>>>> "aaron" == Aaron Bentley <aaron.bentley at utoronto.ca> writes:
aaron> Aaron Bentley has voted -0.
aaron> Status is now: Waiting
aaron> Comment:
aaron> Please try to write a little bit more. I did not find your
aaron> explanation very clear.
Sorry.
aaron> i.e. I did not understand that location was supposed to be a
aaron> parameter, but had not been added.
Obviously a left-over by a previous refactoring, itself
pre-dating the 'create-prefix' patch for the 'init' command (rev
2522) ; _create_prefix has been introduced when the 'push'
command have been refactored. And in the process, 'location' was
orphaned from its natural scope. I just checked that it has the
same name in init and push and fired.
aaron> Also, there should really be a test somewhere.
My thought too :)
aaron> Also, I'm not clear that location is a required parameter. Can't
aaron> you use transport.base?
Frankly, I didn't investigate, it crossed my mind but I thought
that 'location' came from command line when 'transport.base' have
been already processed by Transport so they may differ (encoding
wise).
The proposed patch was just a
'eerk-plug-the-hole-before-the-bug-report' than a
'let-s-do-that-cleanly'.
Vincent
More information about the bazaar
mailing list