Sorry, Valentine. Ralf have mentioned another @staticmethod, I thought it is what you remarks, so there is no that one. Should we reopen another patch to refactor the @staticmethod parse_line? Since I thought these remarks' workload is high, could we?
best regards from Xuguo Wang 2016-07-04 15:15 GMT+08:00 Valentine Sinitsyn <[email protected]>: > Hi Xuguo, > > > On 01.07.2016 08:57, Xuguo Wang wrote: >> >> *** >> All remarks: >> 1 Add a whitespace after the comma ', ' >> done >> 2 # parse /proc/ioports >> omit, self-explanatory >> 3 # region is larger than two bytes >> done >> 4 overlay != overlap... >> done >> 5 Now that I see it: >> I can find these three lines of code six times in this >> patch. Isn't >> there a chance to consolidate? >> It's always the same repeating snippet: >> refactored >> 6 Amend to sth like 'pad with 0xff, if the last address is >> not 0xffff' >> done >> 7 This comment doesn't match to the next line. The next >> line does not >> parse the IOMemRegions, it defines a class... This >> comment is - in my >> opinion - misleading. >> omit, since the code self-explanatory >> 8 E.g., I'm not okay with patch 2/3, as it contains a lot >> of redundant >> code. >> refactor >> 9 Comments help, they are useful and necessary for others >> to understand your code. >> refactor >> >> valentine >> 1 Any reason for using old-style objects instead of 'class >> IORegion(object)' here? >> I think these patch just add the functional of >> pio_bitmap generator. >> 2 Having regionaclass as the first argument here strongly >> suggests you want >> @classmethod, not @staticmethod here. Is there a way to >> refactor your code? >> This could be done in the next patch. > > What do you mean by this quote? I don't see neither changes in your V5, nor > counter-arguments here. > > Valentine > > >> >> jan >> 1 Please only repost updates when you are done with them, >> and it makes >> sense for the reviewer to look at them again. Otherwise, >> they will get >> frustrated and spend their time on other things. >> done >> *** >> >> Xuguo Wang (3): >> tools: Amend the comments. >> tools: Refactor the parse_ports. >> tools: Template refactor. >> >> tools/jailhouse-config-create | 407 >> +++++++++++++++++++++++++++++++++++++----- >> tools/root-cell-config.c.tmpl | 21 +-- >> 2 files changed, 369 insertions(+), 59 deletions(-) >> > -- You received this message because you are subscribed to the Google Groups "Jailhouse" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
