4:17 a.m.
Hi Chris,
Please check the new version of the patch set (when it will be approved
by mailing list moderator because of its size).
Chris Ferron wrote:
Couple of things,
#1 I noticed that the scripts section of Software Settings in need of
Tuning is missing. I see the nice header, but not data.
Done.
#2 C0 active table cells are not aligned , and the CPU's are also
not
aligned correctly in the Processor Idle state report section.
Done. I hope. :-) I
have aligned table headers as in original Powertop
If you mean something else please tell.
#3 I get some warning compiling, I know PowerTOP is not warning less,
but would like to attempt to prevent adding more. (and fixing exiting
ones of course)
report/report-maker.cpp: In destructor ‘report_maker::~report_maker()’:
report/report-maker.cpp:51:10: warning: deleting object of abstract
class type ‘report_formatter’ which has non-virtual destructor will
cause undefined behaviour [-Wdelete-non-virtual-dtor]
report/report-maker.cpp: In member function ‘void
report_maker::setup_report_formatter()’:
report/report-maker.cpp:110:10: warning: deleting object of abstract
class type ‘report_formatter’ which has non-virtual destructor will
cause undefined behaviour [-Wdelete-non-virtual-dtor]
Done.
#4 report[c,h] should probable just go in your new sub/dir (report).
I will send this patch later because it will make my patch bigger and
that could
make the understanding more difficult. It could be a separated patch to
easily
see changes.
#5 csv report has alignment issues for both the cpu Idle and Freq.
Even if they are machine readable, they need to be easily parsed using
open office or MS excel and look aligned with the http report.
CSV has a lots of
questions. First of all, what CSV standard to follow?
Should it be readable by parsers or by Offices?
I have tried to make report maker to be CSV RFC compliant, but as I see
Offices doesn't like that CSV. They both prefer to use semicolon as
separator,
and not to quote strings because of spaces inside of them, and not to quote
empty strings. The only reason to quote is when the string contains
semicolon
inside of it.
So I have tuned CSV report maker to produce CSVs to be read by Offices.
Both (MS and Open) can read it with default settings.
The other question is formatting inside the CSV. In my opinion there
should not
be any space padding for values inside the CSV. First of all, is is a
machine-readable
format. Second, MS Office ignores it totally.
By the way, as I see, both offices uses following text alignment rules:
if the cell
consists of text (letters) then it will be shown aligned to the left,
and if the cell
is a number, then it will be shown aligned to the right. This formatting
rules
are reasonable, so we could totally remove any padding by space in CSV
(and still
have pretty table in Offices).
What you think?
And if we need report in human readable text format, we could create TXT
report
formatter.
--
Best regards,
Igor Zhbanov,
Expert Software Engineer,
phone: +7 (495) 797 25 00 ext 3806
e-mail: i.zhbanov(a)samsung.com
ASWG, Moscow R&D center, Samsung Electronics
12 Dvintsev street, building 1
127018, Moscow, Russian Federation