Code review comment for lp:~osomon/unity-2d/dnd-reorder-apps

Revision history for this message
Ugo Riboni (uriboni) wrote :

> > The only thing that comes to mind make the intent clearer is to do something
> > with http://doc.qt.nokia.com/latest/qcoreapplication.html#postEvent (which
> > will basically send any QEvent derived class to any QObject derived class on
> > the next event loop iteration).
> > I wish I had any better suggestion.
>
> So that would mean messing with C++, i.e. more lines of code and a less
> confined hack.
> Do we agree that the current solution (Timer with triggeredOnStart), even if
> not fully satisfactory, will do for now, until we come up with a more elegant
> one? I can document the hack more extensively if that’s a concern.

Yes please, add some more documentation explaining that what you're really trying to do there is just run something on the next event loop iteration, and that should be enough for now.

> > I will have a look at them to determine if/how they can help simplify the code.
> > The rest of your comments have been addressed (including merging your code re-
> > organisation that makes sense and seems to work, thank you for this).
>
> So I discussed it very briefly with Florian yesterday, and his code in its
> current state only supports incoming drag and drop events, not initiating new
> drag events, which would be needed if we were to reimplement this functionality.
> Those two branches will be better processed separately, and we may choose to
> refactor this one at some point in the future. In the meantime, it should be
> ready for a new (hopefully last) round of review.

I checked it and it looks good to me. Add the comment above while I do a last round of functional review and we should be good to close this and move on :)

« Back to merge proposal