This is an automatically generated e-mail. To reply, visit: http://git.revie= wboard.kde.org/r/102921/

On October 19th, 2011, 10:35 p.m., George K= iagiadakis wrote:
= = =
drkonqi/duplicatefinderjob.h (Diff revision 1)
class DuplicateFinderJob : public KJob
80
        Result data() const<=
/span>;
A more in=
tuitive name for this function would be result() I think ;)

drkonqi/duplicatefinderjob.h (Diff revision 1)
	class DuplicateFinderJob : public KJob
		80	Result data() const<= /span>;

Ok. :)

On October 19th, 2011, 10:35 p.m., George K= iagiadakis wrote:

= = =


class ParseBugBacktraces : QObject
	66	template<class Iter>
	67	ParseBugBacktraces::DuplicateRating rating(Iter it, I= ter itEnd, Iter it2, Iter itEnd2);
	68
	69	template<class Iter>
	70	Iter findCrashStackFrame(Ite= r it, Iter itEnd);
	71
	72	template<class Iter>
	73	Iter increaseIterIfNeeded(It= er it, Iter itEnd);
	74
	75	//TODO improve this stuff, it is just a HACK
	76	template<class Iter>
	77	ParseBugBacktraces::DuplicateRating rating(Iter it, I= ter itEnd, Iter it2, Iter itEnd2)
	78	{
	79	int matches =3D 0;
	80	int lines =3D 0;
	81
	82	it =3D findCrashStackFrame(it, itEnd);
	83	it2 =3D findCrashStackFrame(it2, itEnd2);
	84
	85	while (it !=3D itEnd && it2 !=3D itEnd2) {
	86	if (it->type() = =3D=3D BacktraceLine::StackFrame &&= it2->type() =3D=3D BacktraceLine::StackFrame) {
	87	++lines;
	88	if (it->frameNumber() =3D=3D it2-><= /span>frameNumber() && it->functionName() =3D=3D it2->functionName(= )) {
	89	++matches;
	90	}
	91	++it;
	92	++it2;
	93	continue<= span class=3D"p">;
	94	}
	95
	96	//if iters do not p= oint to emptylines or a stackframe increase them
	97	if (it->type() != =3D BacktraceLine::StackFrame && it->ty= pe() !=3D BacktraceLine::= EmptyLine) { =
	98	++it;
	99	continue<= span class=3D"p">;
	100	}
	101	if (it2-><= span class=3D"n">type() != =3D BacktraceLine::StackFrame && it2->t= ype() !=3D BacktraceLine::EmptyLine) {
	102	++it2;
	103	continue<= span class=3D"p">;
	104	}
	105
	106	//one bt is shorter= than the other
	107	if (it->type() = =3D=3D BacktraceLine::StackFrame &&= it2->type() =3D=3D BacktraceLine::EmptyLine) {
	108	++lines;
	109	++it;
	110	continue<= span class=3D"p">;
	111	}
	112	if (it2-><= span class=3D"n">type() = =3D=3D BacktraceLine::StackFrame &&= it->type() =3D=3D BacktraceLine::EmptyLine) {
	113	++lines;
	114	++it2;
	115	continue<= span class=3D"p">;
	116	}
	117
	118	if (it->type() = =3D=3D BacktraceLine::EmptyLine && = it2->type() =3D=3D BacktraceLine::EmptyLine) {
	119	//done
	120	break;
	121	}
	122	}
	123
	124	if (!lines) {
	125	return ParseBugBacktraces::NoDuplicate;
	126	}
	127
	128	const int rating =3D matches * 100 / lines;
	129	if (rating =3D=3D= 100) = {
	130	return ParseBugBacktraces::PerfectDuplicate;
	131	} else if (rating >=3D 90) {
	132	return ParseBugBacktraces::MostLikelyDuplicate;
	133	} else if (rating >=3D 60) {
	134	return ParseBugBacktraces::MaybeDuplicate;
	135	} else {
	136	return ParseBugBacktraces::NoDuplicate;
	137	}
	138
	139	return ParseBugBacktraces::NoDuplicate;
	140	}
	141
	142	template<class Iter>
	143	Iter findCrashStackFrame(Ite= r it, Iter itEnd)=
	144	{
	145	Iter result =3D std::find_if(it, itEnd, boost<= /span>::bind(&BacktraceLin= e::type, _1) =3D=3D BacktraceLine::KCrash);=
	146	if (result !=3D <= span class=3D"n">itEnd) {=
	147	result =3D std::find_if(result, itEnd<= span class=3D"p">, boost:= :bind(&BacktraceLine= ::type, _1) =3D=3D= BacktraceLine::StackFrame);
	148	}
	149
	150	return result;
	151	}

These fun=
ctions aren't really templates. They kind of rely on Iter being a QList=
<BacktraceLine>::const_iterator. I am ok with making them templates j=
ust for avoiding typing the whole QList<BactraceLine>::const_iterator=
, but actually the same effect could be achieved with a typedef. In any cas=
e, they do not need to be in the header. It makes compilation slower and do=
esn't achieve anything. Just move them in the .cpp, even if you leave t=
hem as templates.

The reason I made them templates was that I had problems compiling t=
he code without. Getting errors like

CMakeFiles/drkonqi.dir/duplicatefinderjob.o: In function `rating(QList<B=
acktraceLine>::const_iterator, QList<BacktraceLine>::const_iterato=
r, QList<BacktraceLine>::const_iterator, Q List<BacktraceLine>:=
:const_iterator)':
/home/mat-not/kde/src/kde-runtime/drkonqi/parsebugbacktraces.h:76: multiple=
 definition of `rating(QList<BacktraceLine>::const_iterator, QList<=
;BacktraceLine>::const_iterator, QList< BacktraceLine>::const_iter=
ator, QList<BacktraceLine>::const_iterator)'
CMakeFiles/drkonqi.dir/parsebugbacktraces.o:/home/mat-not/kde/src/kde-runti=
me/drkonqi/parsebugbacktraces.h:76: first defined here
collect2: ld returned 1 exit status
make[2]: *** [drkonqi/drkonqi] Error 1
make[1]: *** [drkonqi/CMakeFiles/drkonqi.dir/all] Error 2

Unfortunately I haven't sorted these out yet and just used templates in=
stead.

In any case if I leave them templates they have to stay in the header, othe=
rwise it would not compile and if it would it would not be standard conform=
ant.

On October 19th, 2011, 10:35 p.m., George K= iagiadakis wrote:

= = =


class ParseBugBacktraces : QObject
	142	template<class Iter>
	143	Iter findCrashStackFrame(Ite= r it, Iter itEnd)=
	144	{
	145	Iter result =3D std::find_if(it, itEnd, boost<= /span>::bind(&BacktraceLin= e::type, _1) =3D=3D BacktraceLine::KCrash);=
	146	if (result !=3D <= span class=3D"n">itEnd) {=
	147	result =3D std::find_if(result, itEnd<= span class=3D"p">, boost:= :bind(&BacktraceLine= ::type, _1) =3D=3D= BacktraceLine::StackFrame);
	148	}
	149
	150	return result;
	151	}

This is q=
uite clever and I like it, but it adds a dependency on boost, which is not =
an existing dependency of kde-runtime and you haven't added the right c=
hecks in CMakeLists.txt either. I would just replace them with for loops if=
 I were you.

Ah, I thought this was used somewhere else.
I'll change that.

On October 19th, 2011, 10:35 p.m., George K= iagiadakis wrote:

= = =

drkonqi/parsebugbacktraces.cpp (Diff revision 1)
	ParseBugBacktraces::ParseBugBacktraces(const BugReport &bug, QO= bject *parent)
		28	m_parser =3D BacktraceParser::newParser("gdb", this);

Yeah, rea=
listically 99% of backtraces on bugzilla are in gdb format, but there can a=
lso be kdbgwin ones... But I think for the moment we can live with just gdb=
.

Indeed, I simply had no time to also implement the kdbgwin ones. :/<=
/pre>




 On October 19th, 2011, 10:35 p.m., George K=
iagiadakis wrote:
 
  =









 =

 =




 
 
  
   
    drkonqi/reportassistantpages_bugzilla_duplicates.cpp
    

     (Diff revision 1)

    
   
  
 

 
  

   
   void BugzillaDuplicatesPage::analyzedDuplicates(KJob *j)

  
 


  
    
    
    393
        BugReport::Status status=
 =3D m_result.status;
  

 



  unused va=
riable?
 






No it is used below. The only reason I added that was to safe some t=
yping. ;)
If you want I'll remove it in fact.




 On October 19th, 2011, 10:35 p.m., George K=
iagiadakis wrote:
 
  =









 =

 =




 
 
  
   
    drkonqi/reportassistantpages_bugzilla_duplicates.cpp
    

     (Diff revision 1)

    
   
  
 

 
  

   
   void BugzillaDuplicatesPage::analyzedDuplicates(KJob *j)

  
 


  
    
    
    401
                item->setBackground(0, brush);
  

  
    
    
    402
                item->setBackground(1, brush);
  

 



  I take it=
 that you want to color the whole line, right? It would be perhaps a better=
 idea to use item->columnCount() instead of hardcoding.
 






Yes that is what was my intention.
You mean adding a loop over all columns and to do it there? --> I'll=
 do that.




 On October 19th, 2011, 10:35 p.m., George K=
iagiadakis wrote:
 
  =









 =

 =




 
 
  
   
    drkonqi/reportassistantpages_bugzilla_duplicates.cpp
    

     (Diff revision 1)

    
   
  
 

 
  

   
   void BugzillaDuplicatesPage::analyzedDuplicates(KJob *j)

  
 


  
    
    
    408
                    i18nc=
("@label", "Your crash has already been re=
ported as <a href=3D\"%1\">Bug =
%1</a>, which is a <strong>duplicate</strong> of <a hr=
ef=3D\"%2\">Bug %2</a>"<=
/span>, QString::number(<=
span class=3D"n">duplicate), QString::number(parentDuplicate))) +
  

  
    
    
    409
                    '\n'=
; + i18nc("@label", "Only <strong><a href=3D=
\"%1\">attach</a></strong> if =
you can add needed information to the bug report.", QString(<=
span class=3D"s">"attach"));
  

  
    
    
    410
            } else if (BugReport::isClosed(status<=
span class=3D"p">)) {
  

  
    
    
    411
                text =3D (parent=
Duplicate =3D=3D duplicat=
e ? i18nc("@label", "Your crash has already been reported as=
 <a href=3D\"%1<=
/span>\">Bug %1</a&g=
t; which has been <strong>closed</strong>.", QString::number(duplicate)) :
  

  
    
    
    412
                    i18nc=
("@label", "Your crash has already been re=
ported as <a href=3D\"%1\">Bug =
%1</a>, which is a duplicate of the <strong>closed</strong&g=
t; <a href=3D\"%=
2\">Bug %2</a=
>.", QString::number(duplicate), QString::=
number(parentDuplicate)));
  

 



  1) The KU=
IT thing should be "@info:label", iirc.
2) I don't really understand the logic of it. If a report is open or it=
 is RESOLVED/NEEDSINFO, then it can't possibly be marked as a duplicate=
. What am I missing?
 






ad 1) I used "@label" since that was already used in the c=
ode there. There is also a cheatsheet around, not sure if it is correct tho=
ugh: http://people.ubuntu.com/~apachelogger/misc/i18nccheatsheet.png

ad 2) The idea is that there exists already a bug which is either open or R=
esolved/Needsinfo, in that case the user should be asked to provide informa=
tion there if they can. The status of the report itself won't be change=
d, this line should just lead to enhancing the existing report instead of c=
reating a new one, which would in fact be a duplicate.




 On October 19th, 2011, 10:35 p.m., George K=
iagiadakis wrote:
 
  =









 =

 =




 
 
  
   
    drkonqi/reportinterface.h
    

     (Diff revision 1)

    
   
  
 

 
  

   public:

  
 


  
    
    
    75
        void setDuplicateId(uin=
t);
  

  
    
    
    76
        uint duplicateId() cons=
t;
  

 



  Interesti=
ng, looks like this is the only place where a bug report number is a uint. =
(I think uint is the correct type, but everywhere else it's an int...)<=
/pre>
 






I used uint since ReportInterface::attachToBugNumber also used uint.=








 On October 19th, 2011, 10:35 p.m., George K=
iagiadakis wrote:
 
  (Unrelate=
d comment: Since I still consider myself the maintainer of drkonqi, could y=
ou please include me explicitly in the "People" field of review r=
equests related to drkonqi in the future? It makes it easier for me to spot=
 them, since it triggers a filter in my inbox. Thanks.)
 









Ok, sure. :=
)




- Matthias




On October 19th, 2011, 6:58 p.m., Matthias Fuchs wrote:







 
  

Review request for KDE Runtime.
By Matthias Fuchs.


Updated Oct. 19, 2011, 6:58 p.m.






Descripti=
on 
 
 
  
   This patch allows DrKonqi to download the possible duplicate=
 bug reports and parse their backtraces.
If a report turns out as duplicate reporting a new bug is not possible anym=
ore.
Thus the number of duplicates reported via DrKonqi should hopefully decline.

Note: The comparing backtraces part is not that good. Unfortunately I do no=
t have much time to find a good algorithm. In the worst case we could make =
sure that only perfect duplicates are declined, that still should improve t=
he current situation imo.
  
 






Diffs=
 


 drkonqi/CMakeLists.txt (e0c2f6f)

 
drkonqi/bugzillalib.h (1e970a0)

 drkonqi/bugzillalib.cpp (aa558a9)

 
drkonqi/duplicatefinderjob.h (PRE-CREATION=
)

 drkonqi/duplicatefinderjob.cpp (PRE-CREATI=
ON)

 drkonqi/parsebugbacktraces.h (PRE-CREATION=
)

 drkonqi/parsebugbacktraces.cpp (PRE-CREATI=
ON)

 drkonqi/reportassistantdialog.cpp (43731b0=
)

 drkonqi/reportassistantpages_base.cpp (4a2=
5c45)

 drkonqi/reportassistantpages_bugzilla_duplicates.h (b0b4462)

 drkonqi/reportassistantpages_bugzilla_duplicates.cpp (30e861d)

 drkonqi/reportinterface.h (c7f645b)=


 drkonqi/reportinterface.cpp (90e00b3)

 drkonqi/ui/assistantpage_bugzilla_duplicates.ui (1ec034a)



View Diff

drkonqi/reportassistantpages_bugzilla_duplicates.cpp (Diff revision 1)
	void BugzillaDuplicatesPage::analyzedDuplicates(KJob *j)
		393	BugReport::Status status= =3D m_result.status;


void BugzillaDuplicatesPage::analyzedDuplicates(KJob *j)
	401	item->setBackground(0, brush);
	402	item->setBackground(1, brush);


void BugzillaDuplicatesPage::analyzedDuplicates(KJob *j)
	408	i18nc= ("@label", "Your crash has already been re= ported as <a href=3D\"%1\">Bug = %1</a>, which is a <strong>duplicate</strong> of <a hr= ef=3D\"%2\">Bug %2</a>"<= /span>, QString::number(<= span class=3D"n">duplicate), QString::number(parentDuplicate))) +
	409	'\n'= ; + i18nc("@label", "Only <strong><a href=3D= \"%1\">attach</a></strong> if = you can add needed information to the bug report.", QString(<= span class=3D"s">"attach"));
	410	} else if (BugReport::isClosed(status<= span class=3D"p">)) {
	411	text =3D (parent= Duplicate =3D=3D duplicat= e ? i18nc("@label", "Your crash has already been reported as= <a href=3D\"%1<= /span>\">Bug %1</a&g= t; which has been <strong>closed</strong>.", QString::number(duplicate)) :
	412	i18nc= ("@label", "Your crash has already been re= ported as <a href=3D\"%1\">Bug = %1</a>, which is a duplicate of the <strong>closed</strong&g= t; <a href=3D\"%= 2\">Bug %2</a= >.", QString::number(duplicate), QString::= number(parentDuplicate)));

drkonqi/reportinterface.h (Diff revision 1)
public:
	75	void setDuplicateId(uin= t);
	76	uint duplicateId() cons= t;

Descripti= on

Diffs=