[patch] [0.12] remove BzrNewError, cleanup exceptions and users

[John Arbash Meinel]

Martin Pool wrote:

It looks like you addressed all of my comments, but you forgot to update
HACKING.


> === modified file 'HACKING'
> --- HACKING	2006-10-16 04:38:20 +0000
> +++ HACKING	2006-11-02 09:38:02 +0000
> @@ -437,10 +437,44 @@
>  Errors and exceptions
>  =====================
>  
> -Errors are handled through Python exceptions.  They can represent user
> -errors, environmental errors or program bugs.  Sometimes we can't be sure
> -at the time it's raised which case applies.  See bzrlib/errors.py for 
> -details on the error-handling practices.
> +Errors are handled through Python exceptions.
> +
> +We broadly classify errors as either being either the user's fault or our
> +fault.  If we think it's our fault, we show a backtrace, an invitation to
> +report the bug, and possibly other details.  This is the default for
> +errors that aren't specifically recognized as being caused by a user
> +error.
> +
> +User errors are things such as referring to a file or location that
> +doesn't exist.  The user could potentially fix them themselves, so they
> +get a short but helpful message.  User errors either have the
> +``is_user_error`` property set, or are of particular classes such as
> +KeyboardInterrupt.
> +
> +Many errors originate as "environmental errors" which are raised by Python
> +or builtin libraries -- for example IOError.  These are treated as being
> +our fault, unless they're caught in a particular tight scope where we know
> +that they indicate a user errors.  For example if the repository format
> +is not found, the user probably gave the wrong path or URL.  But if one of
> +the files inside the repository is not found, then it's our fault --
> +either there's a bug in bzr, or something complicated has gone wrong in
> +the environment that means one internal file was deleted.
> +
> +Many errors are defined in ``bzrlib/errors.py`` but it's OK for new errors
> +to be added near the place where they are used.
> +
> +Exceptions are formatted for the user by conversion to a string
> +(eventually calling their ``__str__`` method.)  As a convenience the
> +``._fmt`` member can be used as a template which will be mapped to the
> +error's instance dict.
> +
> +New exception classes should be defined when callers might want to catch
> +that exception specifically, or when it needs a substantially different
> +format string.
> +
> +Exception strings should start with a capital letter and should not have a
> +final fullstop.
> +
>  
>  
>  Jargon

...

> === modified file 'bzrlib/tests/__init__.py'
> --- bzrlib/tests/__init__.py	2006-11-02 07:47:25 +0000
> +++ bzrlib/tests/__init__.py	2006-11-02 10:15:49 +0000
> @@ -666,7 +666,7 @@
>              a_callable(*args, **kwargs).
>          """
>          local_warnings = []
> -        def capture_warnings(msg, cls, stacklevel=None):
> +        def capture_warnings(msg, cls=None, stacklevel=None):
>              # we've hooked into a deprecation specific callpath,
>              # only deprecations should getting sent via it.
>              self.assertEqual(cls, DeprecationWarning)
> 

^- Why did you make 'cls' an optional parameter. I was pretty sure that
it was required, because it is required for 'warnings.warn()' which is
are default warning algorithm.

Though I would like to change it so that symbol_versioning.warn is a
*wrapper* around it (which can pass DeprecationWarning by default),
rather than directly being that function.

...


> === modified file 'bzrlib/tests/test_selftest.py'
> --- bzrlib/tests/test_selftest.py	2006-10-29 07:08:01 +0000
> +++ bzrlib/tests/test_selftest.py	2006-11-02 09:38:09 +0000
> @@ -846,7 +846,7 @@
>          sample_test.run(result)
>          self.assertContainsRe(
>              output_stream.getvalue(),
> -            "[1-9][0-9]ms/   [1-9][0-9]ms\n$")
> +            r"\d+ms/ +\d+ms\n$")
>          

^- I'm not sure how this got mixed in with your other changes. I'm okay
with it, but I thought we weren't positive about how lax we wanted to be
about the number of milliseconds it takes to run.


So a couple questions, and 1 request to update HACKING, but otherwise +1
from me.

John
=:->


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 254 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20061102/9dc6e1e1/attachment.pgp