Post-commit review for the new accessibility classes

Amandeep Singh aman.dedman at gmail.com
Wed Sep 26 00:11:07 BST 2012


Hi Frank. Thanks for your patch.
I just pushed the required changes.

On Wed, Sep 26, 2012 at 1:44 AM, Frank Reininghaus <frank78ac at googlemail.com
> wrote:

> Hi Amandeep and Frederik,
>
> I started to go through the new code, fixed a couple of coding style
> issues and removed some unnecessary complexity. The old version of
> KItemListAccessibleCell::navigate() was the most complicated way to
> return -1 I've ever seen ;-)
>
> I'm by no means done, but I already have a few things that I'd like
> you to take care of/comment on:
>
> 1. Your new files have no license header. This is totally unacceptable
> and must be fixed ASAP.
>
> 2. You've wrapped most of the code in #ifndef QT_NO_ACCESSIBILITY, but
> not the #includes for the QAccessible headers. I wonder if these
> headers are actually installed if Qt is built without accessibility or
> if someone who built a non-accessible Qt will get a build error here?
>
> 3. I see that you print some debug output in
> KItemListViewAccessible::cellAt(). Is this still important for your
> current debugging needs? If not, I'd prefer if this was removed.
>
> Best regards,
> Frank
>



-- 
Aman
irc: dedman @ freenode.net
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20120926/56fc9473/attachment.htm>


More information about the kfm-devel mailing list