On 2013/08/01 15:39:18, gz wrote:
> Rietveld has screwed the diff, you'll need to run `lbox propose -cr
-v` again
> which should fix it.
Odd - the diffs show up fine to me. I've re-proposed anyway.
> One comment from looking at the changes on launchpad, this code
ignores a
> potential ErrBadPattern from path.Match() - seems like for user sanity
it might
> be nice to propogate that up to where they can see it. At the least
I'd like to
> see a test that has a borked pattern (unmatched [ or similar should
do) and
> asserts that things don't blow up.
Done. Go's path.Match function checks validity lazily, and I didn't
think it worthwhile expending effort to validate it ahead of time. Thus,
pattern validity will not be checked if it is never considered (i.e. a
preceding pattern matches). I've codified this in a test.
On 2013/08/01 15:39:18, gz wrote:
> Rietveld has screwed the diff, you'll need to run `lbox propose -cr
-v` again
> which should fix it.
Odd - the diffs show up fine to me. I've re-proposed anyway.
> One comment from looking at the changes on launchpad, this code
ignores a
> potential ErrBadPattern from path.Match() - seems like for user sanity
it might
> be nice to propogate that up to where they can see it. At the least
I'd like to
> see a test that has a borked pattern (unmatched [ or similar should
do) and
> asserts that things don't blow up.
Done. Go's path.Match function checks validity lazily, and I didn't
think it worthwhile expending effort to validate it ahead of time. Thus,
pattern validity will not be checked if it is never considered (i.e. a
preceding pattern matches). I've codified this in a test.
https:/ /codereview. appspot. com/12235043/