Shawn Walker wrote: > > - line 2827: here's another instance of something I'm positive I've > > seen before. I know there are significant differences, but we have > > to figure out a way (post FCS) to refactor these functions so that > > they're maintainable. You might want to have a comment here > > explaining that this is a copy of __get_pkg_list() from api.py, > > with some its functionality removed, which is why some constructs > > look weird here. And that changes here should be reflected there > > (and vice versa). > > Yes, there are four very different implementations. While the matching > logic is (almost) identical across them, the behaviour of each one is > not. In particular, the API version relies on constants and data > structures specific to the image, while this version does not. > > I would split the logic into separate subroutines, but that has a > significant performance penalty, so I'm stuck as far as I can tell. I > have attempted to "commonise" them before.
And I'm sure you've explained this to me before, too. I still wish we had a solution for this. > > I think I suggested a long time ago to throw an exception at the > > end of the generator to return things like the matching and > > non-matching patterns here. I wonder if perhaps a better paradigm > > is to return those values through parameters, and if so, why I > > hadn't thought of that (or rejected it) at the time. > > The advantage of the exception approach is that in the event that you > have multiple errors you can easily pass back unmatched pattern data > using the same object you pass back all other data. > > The other advantage I see is that a consumer of the interface has one, > consistent way of handling any errors and one object as the source of > that data. > > However, I admit that it seems rather grotty that we end up with things > like raise_unmatched, etc. and I'm not adverse to reserving exceptions > for exceptional cases while using parameters to return other bits of > data. And now that you have raise_matched, you're raising exceptions for something that isn't even an error; it's just a return value. I think I'd lean in the direction of using return parameters, but I also would wait until after FCS. > As such, I've removed the 'state' field and verified the > 'short_state' field doesn't show up in anything but the default > human-readable and tsv output. Sounds good. You'll need to rework the man page changes in nroff, of course. Danek _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
