On Friday, August 15, 2014 04:12:59 PM Atwood, Matthew S wrote: > Ah, I was wondering what the proper way to do that was, sorry for the > confusion. My go to was to respond to previous version messages as > whether or not the feedback had been added. As far as default behavior > for igt tests, I wouldn't be averse to it, maybe Thomas Wood can chime > in on that?
Generally we add a small not below the commit message similar to this (the exact syntax isn't important), generally above the --- so it remains in the commit log (just an example): v2 - Fixed possible race condition v3 - Updated comments per <commenter> - Add unit test Like I said, the exact form isn't important, just that it's clear and readable. Also, you should check the date on your machine, your emails are dated from 2012, and that means they end up *way* down in my inbox. > > -----Original Message----- > From: Daniel Vetter [mailto:[email protected]] On Behalf Of Daniel Vetter > Sent: Friday, August 15, 2014 4:19 AM > To: Atwood, Matthew S > Cc: [email protected] > Subject: Re: [Piglit] [PATCH v3] programs/run.py add file sync command line > option This isn't really a change to run.py, it's really a change to results.py, you should change your subject line, something like: results.py: Add option to sync after each json write Aside from that This looks good to me, if you could send out a v4 at some point with the subject updated I'd be willing to push that if no one else objects. Did you ever do any profiling to see if this has a significant performance impact to fsync()? If it's not significant it might be just worth turning on universally and having a --no-fsync like Daniel suggested. Either way this looks pretty good to me and if you'll send out a v4 with an updated commit message (addressing both Daniel and I's comments) I'd be happy to merge it for you. [snip]
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
