On Mon, May 31, 2010 at 04:33:57PM +0530, Saurabh Vyas wrote: > Done, the new webrev : http://cr.opensolaris.org/~saurabhv/fix-10437-rev3/
Looks better. The format for your putback comments is incorrect, though. There should be no extra dashes or spaces. You'll need to recommit this workspace and use the following format: 10437 download_done should raise custom error instead of assert It should be the bugid, a single space, and the bug synopsis. > > - 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. > > Sure I would add up the test cases here, but should that be part of > this bug-fix itself ? (as a new test module for progress-tracker is to > be added) Or should I raise another bug for that ? (as I am not sure > about what all scenarios needs to be tested ). I would say that if you're going to the trouble to work on the progress tracker, which is a relatively unmaintained bit of code, that you may as well add the test-case with this putback. You can file an additional bug to add Progress Tracker test cases to the test-suite, and put these back together. However, if you don't integrate tests for this, it's possible that other changes may break your code in the future. Where possible, we request that engineers supply new testcases for code changes that they integrate. -j _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
