Review Request: VcsEventWidget: Fetch only the last n events from the history, request earlier ones as needed

Andreas Pakulat apaku at gmx.de
Sun Jan 17 14:07:46 UTC 2010


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


The emitting of "finished" after resultsReady IMHO totally makes sense. The fact that log() will return not more than the requested amount of revisions, but possibly less also seems logical to me, but feel free to make that explicit in the API docs. Other than that just a few minor remarks:


/trunk/extragear/sdk/kdevplatform/vcs/models/vcseventmodel.cpp
<http://reviewboard.kde.org/r/2638/#comment3122>

    Usually we use "q" for that, but I don't care too much.



/trunk/extragear/sdk/kdevplatform/vcs/models/vcseventmodel.cpp
<http://reviewboard.kde.org/r/2638/#comment3123>

    This would look nice as initialization list.



/trunk/extragear/sdk/kdevplatform/vcs/models/vcseventmodel.cpp
<http://reviewboard.kde.org/r/2638/#comment3124>

    This looks strange, I don't see any elements being inserted here, so why does the model tell the view it got new data?



/trunk/extragear/sdk/kdevplatform/vcs/vcsevent.h
<http://reviewboard.kde.org/r/2638/#comment3121>

    This seems to be an unrelated change, would be good to have that in a separate commit.


- Andreas


On 2010-01-17 13:16:28, Fabian Wiesel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2638/
> -----------------------------------------------------------
> 
> (Updated 2010-01-17 13:16:28)
> 
> 
> Review request for KDevelop and Andreas Pakulat.
> 
> 
> Summary
> -------
> 
> VcsEventModel now has a IBasicVersionControl, so that it can fetch the events as they are requested by the view.
> While it now supports the canFetchMore()/fetchMore() interface of QAbstractItemModel, further elements would
> only be requested, if the bottom has been reached.
> To hide the latency of fetching further events, they are internally prefetched, when a low watermark is reached.
> The fields of the elements, which are currently fetched are returned by the model as "Loading...", so that the user
> gets a feedback, that those fields are not yet available.
> 
> It relies on the following properties, which are not specified by the IBasicVersionControl::log() in that way,
> but happens to be the case with the svn-plugin and (after that patch to DVcsJob) the hg-plugin.
> - log() returns _at least_ "limit" events, or there are fewer events left than requested. This is required,
>    so that the model can answer canFetchMore() correctly, requires only an update of the specification of log().
> - finished() is fired _after_ resultsReady() (patch toDVcsJob)
> 
> VcsEventWidget also works directly with IBasicVersionControl, instead indirectly by getting it from the job.
> 
> Finally, only indirectly related, some const correctess in VcsEvent.
> 
> 
> Diffs
> -----
> 
>   /trunk/extragear/sdk/kdevplatform/vcs/dvcs/dvcsjob.cpp 1075986 
>   /trunk/extragear/sdk/kdevplatform/vcs/models/vcseventmodel.h 1075986 
>   /trunk/extragear/sdk/kdevplatform/vcs/models/vcseventmodel.cpp 1075986 
>   /trunk/extragear/sdk/kdevplatform/vcs/vcsevent.h 1075986 
>   /trunk/extragear/sdk/kdevplatform/vcs/vcsevent.cpp 1075986 
>   /trunk/extragear/sdk/kdevplatform/vcs/vcspluginhelper.cpp 1075986 
>   /trunk/extragear/sdk/kdevplatform/vcs/widgets/vcseventwidget.h 1075986 
>   /trunk/extragear/sdk/kdevplatform/vcs/widgets/vcseventwidget.cpp 1075986 
> 
> Diff: http://reviewboard.kde.org/r/2638/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Fabian
> 
>





More information about the KDevelop-devel mailing list