I'll add a review comment of my own: the use of osnet.publish in usr/src/pkg/transforms/publish is leftover from when this was expressed differently. I should probably get rid of it, and just abort directly from the architecture match.
> My caveat: I'm really quite ill and haven't managed a terribly > coherent or thorough review. But, I'd rather see this go back than > wait on me since we still have a full workspace code review to go, > and I'll also be in there again soon so may have more comments then. Get better. > - Yes, the consolidation dependency should be in the transforms. I > just hadn't gotten around to doing it, so thanks. Thanks for the confirmation. > - Comments in the makefile are good, again thanks. You're welcome, and thanks for the confirmation. > - I'm not at all convinced that we want any incorporation at all for > the 'extra' packages. But, I can't articulate well why right now. Should I leave it or take it out? I don't really like the thought that a package is arbitrarily unconstrained as a function of a legal or political boundary, rather than a technical one. > - For the depend-on-incorporation, I was thinking pkgmogrify would > get a directive that said "if you see this pattern, *add* this > new action" rather than transform the existing action. (Multiple > matches would just add multiple of the same new action.) This would be a significant RFE to pkgmogrify: it would violate the 1:1 input to output line constraint. Currently, a transform may change only the current action, and no other. > - Extensive shell syntax in the makefile does make me nervous. What > about users who use csh and its derivatives? Like me? :) I have tested this with tcsh as a user shell, and can do others if folks want. ISTR John running into an issue with something like that a while back, with one of the (thankfully) temporary hacks. John, what shell do you use? > That's it so far. Thanks. --Mark _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
