On Mon, Aug 08, 2011 at 11:38:19AM -0700, Shawn Walker wrote: > On 08/08/11 11:21, Edward Pilatowicz wrote: > >On Sun, Aug 07, 2011 at 07:39:33PM -0700, Brock Pytlik wrote: > >>On 08/05/11 16:32, Edward Pilatowicz wrote: > >>>On Tue, Aug 02, 2011 at 06:37:22PM -0700, Brock Pytlik wrote: > >>>>On 07/22/11 14:25, Brock Pytlik wrote: > >>>>>Webrev: > >>>>>https://cr.opensolaris.org/action/browse/pkg/bpytlik/18439-v1 > >>>>> > >>>>New webrev: > >>>>https://cr.opensolaris.org/action/browse/pkg/bpytlik/18439-v2 > >>>> > >>>sorry for the delay. > >>>i still need to review the tests. > >>>comments on the code are below. > >>>ed > >>> > >>>---------- > >>>general > >>> > >>>- i thought that the last time we talked about this we had agreed on > >>> putting a copy of the "parsable_version" into the progress tracker > >>> class? if we did this we wouldn't have to add an extra parameter to > >>> all the linked image functions, and we wouldn't have to scan the > >>> arguments in __pkg_cmd(). > >> > >>I don't remember agreeing on that, I remember discussing it. When I > >>got back and looked at the code more, I decided that I didn't like > >>it inside the progress tracker since it doesn't have anything to do > >>w/ the ProgressTracker, and just stuffing it inside didn't strike me > >>as right. > >> > > > >we've got verbose and quiet in there, so i think parsable fits pretty > >well, i also think it simplify the code a bunch because you wouldn't > >need to add new parameters to so many functions. > > I have to agree with Brock; it makes little sense to me that have > this in ProgressTracker. It isn't for monitoring progress -- it's > purely a parsable version of the plan. >
fyi, i don't want the actual parsable output functionality in ProgressTracker. i just wanted a variable indicating that we're executing in parsable output mode in the ProgressTracker. basically, i view the ProgressTracker as a class that allows the api client to tell the api how it wants output to be managed and get detail notification of events as that happens (and what it does with those events is up to it). i put the verbose and quiet variable flags into the ProgressTracker class because in the case of linked images we have to convey that information onto subprocesses, this seemed a logical extension of that. but really it's not that important. not bundling the two just means we add an extra parameter to a bunch of code paths that already pass a progresstracker pointer around. ed _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
