[PATCH RFC] scmutil: add a simple key-value file helper

Augie Fackler raf at durin42.com
Mon Dec 5 18:13:14 EST 2016


> On Dec 5, 2016, at 17:51, Jun Wu <quark at fb.com> wrote:
> 
> Excerpts from Augie Fackler's message of 2016-12-05 17:07:47 -0500:
>> 
>>> On Dec 5, 2016, at 16:54, Jun Wu <quark at fb.com> wrote:
>>> 
>>>> +                e = _("Invalid value in a simple key-value file")
>>>> +                raise self.InvalidValueInFileException(e)
>>> 
>>> These sound like a programmer error. Maybe just raise RuntimeError.
>> 
>> I'm not comfortable with that, FYI. RuntimeError IMO should be reserved for errors in the Python runtime itself, not run-time errors caused by programmer error.
> 
> From what I have learn from the code base. We use RuntimeError for
> programmer errors. Seems to be the most "popular" solution.

Past mistakes don't convince me that we should continue doing this. :)

> 
>  merge.py
>  317:            raise RuntimeError("localctx accessed but self._local isn't set")
>  323:            raise RuntimeError("otherctx accessed but self._other isn't set")
> 
>  context.py
>  2011:           raise RuntimeError('can\'t reuse the manifest: '
>  2014:           raise RuntimeError('can\'t reuse the manifest: '
> 
>  localrepo.py
>  1019:           raise RuntimeError('programming error: transaction requires '
> 
> There are some subclasses of RuntimeError though:
> 
>  bundle2.py
>  298:class TransactionUnavailable(RuntimeError):
>  856:                raise RuntimeError('duplicated params: %s' % pname)
>  909:            raise RuntimeError('part can only be consumed once')
>  1078:        raise RuntimeError('no repo access from stream interruption')
> 
>  error.py
>  149:class LockInheritanceContractViolation(RuntimeError):
>  168:class PushRaced(RuntimeError):
>  198:class ReadOnlyPartError(RuntimeError):

I'd rather see a new ProgrammingError or something in error.py, and have it inherit from RuntimeError for six months or so and then cut the cord on this silliness.

(I know this opinion of mine is not widely held, but I'd also rather we /at least/ introduce new specific exceptions rather than just blindly reusing something this generic (note that if you catch RuntimeError, you're also transitively catching NotImplementedError, which is probably not what you had in mind...)



More information about the Mercurial-devel mailing list