Review Request 117379: Various fixes for mercurial plugin VCS.

[Dāvis Mosāns]

> On April 5, 2014, 1:20 p.m., Aleix Pol Gonzalez wrote:
> > tests/initTest.cpp, line 174
> > <https://git.reviewboard.kde.org/r/117379/diff/1/?file=262921#file262921line174>
> >
> >     Jobs should delete themselves after they're done.

So no need to delete them? I'm just not really familiar with KDE. I see it's inherited from KJob. I'll remove unneeded deletes.


> On April 5, 2014, 1:20 p.m., Aleix Pol Gonzalez wrote:
> > kdevmercurial.desktop, line 75
> > <https://git.reviewboard.kde.org/r/117379/diff/1/?file=262910#file262910line75>
> >
> >     You can change this to how we're doing it with the rest of the plugins, using configure_file.
> >     This way we won't need to maintain this field anymore.

Will update.


On April 5, 2014, 1:20 p.m., Dāvis Mosāns wrote:
> > All in all, the patch looks good, but there's lots of changes in the coding style so it's hard to review the rest. I would suggest you to just push it, though, and then iterate over it. I don't think much people is using it anyway, if it still had the 14 version number...

Yes, I don't really like that here it's using it all in single patch. I've it in separate commits for me.


- Dāvis


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117379/#review55020
-----------------------------------------------------------


On April 5, 2014, 6:09 a.m., Dāvis Mosāns wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117379/
> -----------------------------------------------------------
> 
> (Updated April 5, 2014, 6:09 a.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdev-mercurial
> 
> 
> Description
> -------
> 
> Various fixes for mercurial support. Basically updated so it can be built against latest kdevplatform. Also fixed tests so they pass.
> 
> 
> Changes:
> 
> 1. Updated formatting
> 
>   * Remove trailing whitespace
>   * Replace tabs with spaces
>   * Reformat code so it's properly indented
> 
> 2. Removed DVCS view factory
> 
>   * Kdevplatform change commit 41db303595d55d355a80d896a25543f449819d30
> 
> 3. Changed MercurialHeadsModel constructor to match VcsEventsWidget
> 
>   * Kdevplatform change commit a6d910861906365f0ef11b99badbbde01e6f3c7b
> 
> 4. Fixed some memory leaks
> 
> 5. Fixed tests
> 
> 6. Other changes
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt c4f91a394215c8401579af38feba4982bed055dc 
>   cmake/FindKDevPlatform.cmake 1a771c52510cb3e71c19b8a1c84680b6a3c64d08 
>   kdevmercurial.desktop ed56b3554686501ac19dabd230f4a01456bc4a2c 
>   mercurialplugin.h 11c3cf203f5a52b6548f4919f5518dd22581d3b5 
>   mercurialplugin.cpp 59198e8b1e3f90c61373f05595eeebba8408de52 
>   mercurialpushjob.h 818043f457ad68aa4f8c87a1595de2e60ecf3711 
>   mercurialpushjob.cpp 0cb79768870d4e0233be5f875a49cd439919aa76 
>   models/mercurialheadsmodel.h 8e2ed09dbac64d03af68d936365adbbea41a58a9 
>   models/mercurialheadsmodel.cpp 41e6c7bbfcabc437932faf8d48cf73e831d03080 
>   models/mercurialqueueseriesmodel.h bd49e5becec5e9c9ce0b420655e0ea48926864f1 
>   models/mercurialqueueseriesmodel.cpp 4fb1d5289edd2b9b699815e889a0f45352dea35a 
>   tests/CMakeLists.txt 2c0a7b95d55a83af7e86622dbdb9401daef98d4c 
>   tests/initTest.h 83ae8efe7d12f7ec2aaeb70c8b9da174f1a89ac1 
>   tests/initTest.cpp fd7eef079e3c3c752815350764ebf34dc4c36715 
>   ui/mercurialheadswidget.h 7eaa6e53fd83fc634e81fcc2b8cd80011ff6e483 
>   ui/mercurialheadswidget.cpp b0e2eb0f5f4968ebb54b2d95a88eba4a9b93f653 
>   ui/mercurialqueuesmanager.h 6411ea5fba9c59732cc82f100eb1c8fd49deee42 
>   ui/mercurialqueuesmanager.cpp 0ebc0e01880af7ae3cf332880852cd57c470eafa 
> 
> Diff: https://git.reviewboard.kde.org/r/117379/diff/
> 
> 
> Testing
> -------
> 
> All tests pass, but they don't cover all functionality.
> 
> Manually verified that adding files, commits does work. Also commit history viewing works fine. Didn't tried remotes nor branches. They might not or might work in some cases.
> 
> 
> I consider this in Alpha quality so basically it can be used to play a bit, but would require more work to be finished.
> 
> 
> Thanks,
> 
> Dāvis Mosāns
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20140405/6c10357a/attachment.html>