On 04.07.2016 18:06, charles king wrote:
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?
Yes, it's okay to postpone the refactoring.
But what's wrong with docstrings? They are trivial.
Valentine
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.
