Code review comment for lp:~axwalk/juju-core/lp1121916-juju-status-filters

Revision history for this message
Andrew Wilkins (axwalk) wrote :

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/

« Back to merge proposal