Merge lp:~garyvdm/qbzr/viz into lp:~qbzr-dev/qbzr/trunk

Proposed by Gary van der Merwe
Status: Merged
Merge reported by: Gary van der Merwe
Merged at revision: 331
Proposed branch: lp:~garyvdm/qbzr/viz
Merge into: lp:~qbzr-dev/qbzr/trunk
To merge this branch: bzr merge lp:~garyvdm/qbzr/viz
Reviewer Review Type Date Requested Status
Alexander Belchenko Approve
Lukáš Lalinský (community) Approve
QBzr Developers Pending
Review via email: mp+496@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Alexander Belchenko (bialix) wrote :

Please, give some hints/comments about your changes. Maybe you discussed this before with Lukáš, but I'm certainly missed it.

Revision history for this message
Gary van der Merwe (garyvdm) wrote :

Sorry - I should have given more comments. I have not dicussed this with Luks.

The search in qlog currently uses a QSortFilterProxyModel, which visits all revisions to see if the search string matches for the selected role.

For revision id's and numbers, indexes are created on load, as they are needed in creating the graph.

This change uses these indexes if you are searching on revision number or revsions id to find the revisions you are looking for.

It behaves a bit differently. Previously, when you search, the revisions that don't match are hidden. With this change, when you search on revision id or revision number, it selects the revision that matches. This is ok because there will only be one revision that matches. In fact, I think it is better.

I am concerend that because there is a difference between in the behaviour between searching on Message or Author, and searching on revision id, and revision number, that it may confuse the user.

Revision history for this message
Lukáš Lalinský (luks) wrote :

Looks fine to me. Most of the time I search for a revision, I need to see it in the context so this makes it even easier than filtering && clearing the filter.

review: Approve
Revision history for this message
Alexander Belchenko (bialix) wrote :

I agreed with Lukáš

review: Approve

Subscribers

People subscribed via source and target branches