On 08/08/11 14:30, Edward Pilatowicz wrote: > 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.
That makes sense, so I'll leave it between you two. -Shawn _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
