[Fwd: Re: RFC: ERROR: Unsupported branch format (improve message)]

[John Arbash Meinel]

...

>>
>>  class UnsupportedFormatError(BzrError):
>>
>> -    _fmt = "Unsupported branch format: %(format)s"
>> +    _fmt = "Unsupported branch format: %(format)s\nPlease run 'bzr upgrade'"
>>
>>
>>  class UnknownFormatError(BzrError):
> 
> +1, but i would also remove the word "branch" from the message since
> in Jari's case it's a repository not a branch (i think).
> 
> Also in general these would be clearer if they give the path - people
> might wonder how init can complain  about an existing format;
> presumably because of a containing bzrdir.
> 

Are you thinking of the 'Warning Format out of date, you should upgrade'
comments?

This is more for 'bzr status' or 'bzr branch' from a 0.0.4 format branch
(gzipped text files).

I looked into adding a url parameter, but the way the code is
structured, it requires a bit of work to pass everything around.
(BzrDir._check_supported only gets a format parameter, and whether
'unsupported' formats are allowed, it doesn't know the url).

If it is important we can fix things. But I felt UnsupportedFormat is
rare enough that it isn't worth messing with.


Oh, and your 0.15rc3 changes also have a small problem with remote
working trees.

It seems they check that the format needs to be upgraded *before* it
checks whether the WorkingTree is NotLocalUrl.

Which means if someone else's branches have working trees, you can get:

bzr: warning: you should run 'bzr upgrade http://someone/elses/branch'

IMO the upgrade warning should be done inside the 'format.open()' rather
than in _check_supported(). for that reason.

Oh, and a few other comments about the 0.15 branch.

1) Are you planning on merging the changes to bzr.dev any time soon? I'm
not sure exactly what hasn't been merged. But all of the tree upgrade
recommendations are not in 'bzr.dev'.

2) Why did you chose this indenting:
    def _check_supported(format, allow_unsupported,
        recommend_upgrade=True,
        basedir=None):

I personally prefer:
    def _check_supported(format, allow_unsupported,
                         recommend_upgrade=True,
                         basedir=None):

It isn't a big deal, but we should try to be consistent. (I think PEP8
gives an example of the latter form, but I didn't see it explicitly stated)

John
=:->