Code review comment for lp:~parthm/bzr/300062-invalid-pattern-warnings

Revision history for this message
John A Meinel (jameinel) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin Pool wrote:
> On 21 June 2010 16:46, Robert Collins <email address hidden> wrote:
>> So, lets go with an error based approach for now, for the following reasons:
>> - its a very small change compared to the current code (wrap the
>> regex compiler to know its taking a list in and print the errorneous
>> pattern out)
>> - it seems genuinely better for some use cases like add
>> - we can add a warning based one later if this feels insufficient.
>>
>> In general I do think its better to stop early and cleanly in commands
>> that can output lots of stuff - if we show a lot then users will often
>> miss the fine print.
>>
>> I'll review the error based one again to move things forward.
>
> I said previously that I slightly preferred a warning if it was
> equally easy to implement. But for the sake of simplicity and of
> getting an error in cases where we do need it, an error is fine with
> me.
>

We've had a bad habit in the past of falling over on trivial details and
preventing users from getting stuff done. I'm not saying that is the
case here, and as always there is a balance to be struck.

As mentioned, this can't be considered a regression, since we used to
raise an unhelpful exception, so raising a helpful one is strictly
better than what we had.

So certainly, land the update (I have not reviewed the code). I'm not as
convinced that failing early is always the best answer, it is probably
ok here.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkwg10gACgkQJdeBCYSNAAPBLwCgokPBGQCdKuxiom9uOmK2DKmN
Q0UAoKYen2O25wLOuQRdvqyUVRV42iLn
=Qoef
-----END PGP SIGNATURE-----

« Back to merge proposal