Code review comment for lp:~parthm/bzr/300062-2.2-bad-pattern-error-part-2

Revision history for this message
Parth Malwankar (parthm) wrote :

> 43 + TYPE_FULLPATH = 1
> 44 + TYPE_BASENAME = 2
> 45 + TYPE_EXTENSION = 3
>
> This looks a bit C-ish, why don't you just use regular keys ('fullpath',
> 'basename' and 'extension') ?
>
> From a design point of view it looks like you're defining several set of
> objects associated with the same key which sounds like you want a dict or even
> a registry...
>

I have updated the patch to use a dict of dict with strings. Its much cleaner now.

> Also, while this is certainly out of scope for this proposal, keep in mind
> that some plugins want to define their own '.xxx-ignore' files and our current
> API is already a pain to use, it would be nice if we can help them by
> providing the right set of function/classes to process a set of paths against
> a given set of patterns coming from a file (or several in bzr case...).

Makes sense. I think it may be possible to build an API on top of the
current implementation to make it easy. I will look into bzr-upload as
suggested by you on IRC.

Thanks for the review.

« Back to merge proposal