Code review comment for lp:~parthm/bzr/300062-better-handling-for-invalid-ignore-pattern

> So, this is comprehensive, and I like that. I have a general point,
> and a couple of specifics. I haven't done a line by line reading [yet,
> I will after the general point is discussed].
>
> Firstly, the general point. I think its fine, if the ignore file is
> damaged to just stop. add doesn't need to be able to succeed when the
> environment is broken. Stop (let the exception propogate) and have the
> user fix the issue. Better that than adding something they wanted
> ignored, I think. Similarly for status.
>

I have update `status` to fail as well. `add` was already raising an
exception for invalid pattern. Making it consistent does make the
code cleaner.

> Secondly, the specific issue - to expose the real re compile function,
> just assign it to a variable that can be accessed: wrappers of the
> form:
> def a_foo(param):
> result = real_foo(param)
> return result
>
> can be rewritten as
> def a_foo(param):
> return real_foo(param)
>
> which is precisely equivalent to
> a_foo = real_foo
>
>
> So in this case, just call the _real_re_compile function, as is. No
> changes needed.
>

I was reluctant to use _real_re_compile as it was private. Its now
renamed to real_re_compile and used directly.

> sidenote: I rather suspect theres a bunch of edits here we may end up
> not wanting, and perhaps we'll want to flatten the history to avoid
> that overhead.
>

I am not sure I understand. Do you mean using bzr-rewrite?

> -Rob

Thanks for the review.