***
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.
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(-)
--
2.5.0
--
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.
