On 04.07.2016 18:28, charles king wrote:
Ok,I think the docstrings are very good, but if I add the docstrings
in my patch only,
the old remained, that is strange, maybe we need a special patch, used for
adding all the class docstrings, Maybe could be done in transplanting
the python 2.7
to the python 3.0. what you think Valentine? Or just add myself added
class only?
I don't think you should rework all the docstring in this patch series.
However it doesn't make much sense to introduce new future refactoring
spots either.
So I'd suggest to convert comments to docstrings in this patch, whenever
appropriate.
Ralf, do you agree?
Valentine
best regards
from Xuguo Wang
2016-07-04 21:11 GMT+08:00 Valentine Sinitsyn <[email protected]>:
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.
