On Wed, Nov 25, 2015 at 06:32:25AM -0800, [email protected] wrote: > > On Tue, Nov 24, 2015 at 04:14:23AM -0800, [email protected] > > wrote: > > > > > I don't like the aproach of passing the returncode through every level > > of piglit at all. I'd much rather use an exception for this, either by > > extending PiglitFatalError or by adding a new exception, and then > > catching it either in the exceptions.handler decorator, or if we have to > > in run/resume. > > Understood, I will apply exception approach then. > > > > > I've got a couple more comments inline. > > > > Also, please figure out where the line-wrapping is coming from and > > remove it (if you're pasting this into a window in your email client you > > will get line-wrap which breaks git am and git apply). Using git > > send-email is really the best way to do this. > > I should have fix issue on my side now. > > > [snip] > > > >> + > >> + def reboot_is_needed(self): > >> + """ Simply return the reboot_needed variable state """ > >> + return self._reboot_needed > > > > This is what a property is for: > > @property > > def reboot_needed(self): > > return self._reboot_needed > > > > Then you access the value as an attribute rather than as a function. > > > > If self.reboot_needed: > > print('reboot now!') > > Thanks for highlighting this :) > > > > > There is a ton of duplication between LinuxDmesg and > > LinuxMonitoredDmesg. I haven't looked too closely, but it seems like > > LinuxMonitoredDmesg could inherit from LinuxDmesg? > > LinuxMonitoredDmesg is quite close but couldn't inherit directly (or may > need to refactor LinuxDmesg code). In addition to look for certain > patterns in dmesg, this class doesn't replace pass with dmesg-warn, and > warn and fail with dmesg-fail, like LinuxDmesg class is doing. > But following various review & comments, I am going to modify the approach > to allow pattern definition and as well as defining where & how to look > for these patterns.
Okay, I'll look again when the next version arrives then. > > > > > [snip] > > > > Dylan > > > Thanks Dylan for your feedback. > > Yann Thanks for this, it sounds pretty useful.
signature.asc
Description: PGP signature
_______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
