Code review comment for lp:~uriboni/webbrowser-app/qml-tabs-model

Revision history for this message
Olivier Tilloy (osomon) wrote :

There is one extraneous blank line in updateCurrentTab, can it be removed?

In insert(), if index is invalid, shouldn’t the fuction return early after printing out the warning? And in test_shouldNotInsertTabAtInvalidIndex, you should also verify that currentIndex and currentTab remain unchanged.

Should we really allow setting currentIndex to -1 when there’s at least one tab in the model? What real-world use case does this cover?

review: Needs Fixing

« Back to merge proposal