[rekonq] Re: Review Request: GCI Task: Copy Context Menu for Network Analyzer

[Pierre Rossi]

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



src/analyzer/networkanalyzer.cpp
<http://git.reviewboard.kde.org/r/100198/#comment402>

    Style: I can haz alphabetical order plz?



src/analyzer/networkanalyzer.cpp
<http://git.reviewboard.kde.org/r/100198/#comment401>

    Weird match for the signals/slots parameters. :S
    Actually this connection is most likely spitting some error at run time since contextMenuEvent isn't even a slot.
    
    I think what you might want to do is 
    set the policy to Qt::CustomContextMenu and then declare your own slot - e.g. void popupContextMenu(const QPoint &pos) - in which you'd put the logic you have below. It's nicer that way than hacking into the contextMenuEvent directly IMHO.
    
    See:
    http://doc.qt.nokia.com/4.7/qwidget.html#contextMenuPolicy-prop



src/analyzer/networkanalyzer.cpp
<http://git.reviewboard.kde.org/r/100198/#comment403>

    Coding style: missing a space between "if" and '(' and a space before and after the operator ">="



src/analyzer/networkanalyzer.cpp
<http://git.reviewboard.kde.org/r/100198/#comment404>

    Style: tabs 'til line 90 should be spaces.


- Pierre


On 2010-12-12 19:07:14, Furkan Üzümcü wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100198/
> -----------------------------------------------------------
> 
> (Updated 2010-12-12 19:07:14)
> 
> 
> Review request for rekonq.
> 
> 
> Summary
> -------
> 
> * Copy context menu for Network Analyzer.
> 
> 
> Diffs
> -----
> 
>   src/analyzer/networkanalyzer.h 9e38663 
>   src/analyzer/networkanalyzer.cpp c5b0883 
> 
> Diff: http://git.reviewboard.kde.org/r/100198/diff
> 
> 
> Testing
> -------
> 
> Tested and works cool!
> 
> 
> Thanks,
> 
> Furkan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/rekonq/attachments/20101212/f178f144/attachment-0001.htm