On Fri, May 28, 2010 at 06:11:16PM +0530, Saurabh Vyas wrote: > Question : Do we still need to log the errors here ? Or just raising > the exception would do ?
I'm not certain. We want to make sure that the exception is saved in the pkg history, but I don't know if that requires the logger for that to happen or not. Shawn would know for sure. > New Webrev : http://cr.opensolaris.org/~saurabhv/fix-10437-rev2/ client.py: - You haven't caught all of the cases where this can be raised. Double-check to make sure that you've found all of the places were we call imageplan.prepare(). It looks to me like the image.repair() code calls imageplan.prepare() too. api_errors.py: - lines 653-655: If you're always going to pass all of the arguments to the function, do you really need to declare keyword arguments here? - line 665, 669, 674, 679, and 681: This could be simplified if you set: res = "" then you can do: res += s instead, and just return res progress.py: - lines 212-218: Same question as with api_errors: If you're always passing all arguments, do you really need keyword arguments here? General questions: - How have you tested this code? Would you please add some api tests to ensure that this works now, and isn't broken later on? There don't appear to be any api tests for the progress tracker right now, so you may have to devise an API module that tests different parts of the progress tracking module. Thanks, -j _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
