[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