Code review comment for lp:~vila/bzr/920455-ssl-defaults

Revision history for this message
Vincent Ladeuil (vila) wrote :

> This is why I was picky about the error message given for the original
> change... unfortunately it's now just a ValueError and traceback due to the
> default config value being bad. This branch changes one of those but not the
> other.

No, please do try again (with this proposal):

  bzr config --remove launchpad_username
  bzr config ssl.ca_certs=/I-dont-exist

vila:~/src/bzr/bugs/920455-ssl-defaults :) $ ./bzr info lp:bzr
bzr: ERROR: Bad value "/I-dont-exist" for option "ssl.ca_certs".

So you get a proper error message that we may want to clarify but you don't
get a traceback.

You got a traceback prior to this change because default_ca_certs() was
raising it which is why I changed the implementation as ValueError is caught
for the *_from_store() functions, not the _default() functions.

I could change the ConfigValueError message to include 'See bzr help
ssl.ca_certs' but we froze the strings for 2.5 so I'd rather do that for
trunk.

I'll file bugs for windows and osx installers to make sure a bundle is
included.

« Back to merge proposal