Review Request: DrKonqi parses bactraces to find duplicates

[George Kiagiadakis]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102921/#review7501
-----------------------------------------------------------


Wow, that took me a while to review...
Ok, first of all, thanks for the patch. I have a few comments to make, the rest of it looks really good.



drkonqi/duplicatefinderjob.h
<http://git.reviewboard.kde.org/r/102921/#comment6486>

    A more intuitive name for this function would be result() I think ;)



drkonqi/parsebugbacktraces.h
<http://git.reviewboard.kde.org/r/102921/#comment6485>

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



drkonqi/parsebugbacktraces.h
<http://git.reviewboard.kde.org/r/102921/#comment6483>

    This is quite 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 checks in CMakeLists.txt either. I would just replace them with for loops if I were you.



drkonqi/parsebugbacktraces.cpp
<http://git.reviewboard.kde.org/r/102921/#comment6482>

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



drkonqi/reportassistantpages_bugzilla_duplicates.cpp
<http://git.reviewboard.kde.org/r/102921/#comment6480>

    unused variable?



drkonqi/reportassistantpages_bugzilla_duplicates.cpp
<http://git.reviewboard.kde.org/r/102921/#comment6478>

    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.



drkonqi/reportassistantpages_bugzilla_duplicates.cpp
<http://git.reviewboard.kde.org/r/102921/#comment6479>

    1) The KUIT 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?



drkonqi/reportinterface.h
<http://git.reviewboard.kde.org/r/102921/#comment6481>

    Interesting, 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...)


(Unrelated comment: Since I still consider myself the maintainer of drkonqi, could you please include me explicitly in the "People" field of review requests related to drkonqi in the future? It makes it easier for me to spot them, since it triggers a filter in my inbox. Thanks.)

- George Kiagiadakis


On Oct. 19, 2011, 6:58 p.m., Matthias Fuchs wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102921/
> -----------------------------------------------------------
> 
> (Updated Oct. 19, 2011, 6:58 p.m.)
> 
> 
> Review request for KDE Runtime.
> 
> 
> Description
> -------
> 
> 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 anymore.
> Thus the number of duplicates reported via DrKonqi should hopefully decline.
> 
> Note: The comparing backtraces part is not that good. Unfortunately I do not 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 the 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-CREATION 
>   drkonqi/parsebugbacktraces.h PRE-CREATION 
>   drkonqi/parsebugbacktraces.cpp PRE-CREATION 
>   drkonqi/reportassistantdialog.cpp 43731b0 
>   drkonqi/reportassistantpages_base.cpp 4a25c45 
>   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 
> 
> Diff: http://git.reviewboard.kde.org/r/102921/diff/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Matthias Fuchs
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20111019/fe44154a/attachment.htm>