Re: [Monotone-devel] review of nvm.connection_info_cleanup

[Stephen Leake]

Thomas Keller <address@hidden> writes:

> Am 17.06.2010 01:32, schrieb Stephen Leake:
>> Thomas Keller <address@hidden> writes:
>> 
>>> The branch nvm.connection_info_cleanup should be feature-complete from
>>> my side, all tests pass and the docs have been updated accordingly.
>>> Please read over it and give me feedback if you find gotchas or think of
>>> other improvements.
>> 
>> I reviewed monotone.texi, but not the code.
>> 
>> The documentation is pretty clear. However, I think in this syntax:
>> 
>> mtn automate pull [--set-default] address@hidden address@hidden [...] 
>> address@hidden
>> 
>> the 'glob' should only be there if the 'uri-or-address' is the old style
>> host address? If true, I think it would be better to list this as two lines:
>> 
>> mtn automate pull [--set-default] address@hidden [...] address@hidden
>> 
>> mtn automate pull [--set-default] address@hidden address@hidden [...] 
>> address@hidden
>> 
>> Then it is easier to say the second one is deprecated. Similarly for
>> push and sync, of course.
>
> Yes, you're right, I'll change that accordingly.

Looks good.

> Are you sure you have the latest revision? These two progress messages
> should no longer come up.

Sorry, I did a sync but not an update. Looks good now.

>> This would be better if there was an explicit "you specified a
>> deprecated argument" message first.
>
> I can add one.

Looks good.

>> In addition, the default-server and known-server db vars are now
>> inconsistent:
>> 
>> database: default-server monotone.ca
>> known-servers: monotone.ca 3e6f5225bc2fffacbc20c9de37ff2dae1e20892e
>> known-servers: mtn://monotone.ca 3e6f5225bc2fffacbc20c9de37ff2dae1e20892e
>> 
>> Is this intended?
>
> Lets say, its expected. I don't know how how I should properly address
> this issue, i.e. if I should write a database migration which only
> touches the _data_, not the schema. One could of course add some code to
> handle this deprecated old variables and change them accordingly, but I
> don't know if that is such a good idea (spaghetti, spaghetti!).

I wasn't so much worried about the old 'known-servers'; I can see that
would be hard to clean out. But the name for the server in the
'database' entry is not the same as the name for the server in the new
'known-servers' entry. That seems confusing. But it's not something
users should have to deal with, so it probably doesn't matter.


>> With this current ssh: command line:

ssh: and file: both work properly with the old (and new) syntax.

-- 
-- Stephe