[Kde-pim] Review Request: Add support to backup a PostgreSQL database

[Tobias Koenig]

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



trunk/KDE/kdepim/runtime/tray/backup.h
<http://reviewboard.kde.org/r/3350/#comment4145>

    Please sort the Qt includes in alphabetical order



trunk/KDE/kdepim/runtime/tray/backup.h
<http://reviewboard.kde.org/r/3350/#comment4147>

    Use meaningful method names, in this case dumpDatabase



trunk/KDE/kdepim/runtime/tray/backup.h
<http://reviewboard.kde.org/r/3350/#comment4146>

    We don't use getXYZ naming scheme in KDE, should be renamed to requiredExecutables()



trunk/KDE/kdepim/runtime/tray/backup.cpp
<http://reviewboard.kde.org/r/3350/#comment4148>

    Can be changed to 'const QString exe = ...'



trunk/KDE/kdepim/runtime/tray/backup.cpp
<http://reviewboard.kde.org/r/3350/#comment4149>

    Please add white spaces between the outer parentheses



trunk/KDE/kdepim/runtime/tray/backup.cpp
<http://reviewboard.kde.org/r/3350/#comment4150>

    const QString dumpFilename ...



trunk/KDE/kdepim/runtime/tray/backup.cpp
<http://reviewboard.kde.org/r/3350/#comment4151>

    Can be written short as
     return QSet<QString>() << "bzip2" << "tar";



trunk/KDE/kdepim/runtime/tray/backupassistant.cpp
<http://reviewboard.kde.org/r/3350/#comment4152>

    white spaces in parentheses



trunk/KDE/kdepim/runtime/tray/backupassistant.cpp
<http://reviewboard.kde.org/r/3350/#comment4153>

    dito...



trunk/KDE/kdepim/runtime/tray/global.h
<http://reviewboard.kde.org/r/3350/#comment4154>

    alphabetic sorting



trunk/KDE/kdepim/runtime/tray/global.cpp
<http://reviewboard.kde.org/r/3350/#comment4155>

    white spaces



trunk/KDE/kdepim/runtime/tray/global.cpp
<http://reviewboard.kde.org/r/3350/#comment4156>

    remove space after '!', same 3 lines below


- Tobias


On 2010-03-23 01:57:43, Brian DeRocher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3350/
> -----------------------------------------------------------
> 
> (Updated 2010-03-23 01:57:43)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> Created derived classes BackupMySQL and BackupPostgreSQL of Backup.  Most of work remains in Backup.  Required executables becomes a set of strings.  Derived classes may append to the set.  MySQL uses mysqldump.  PG uses pg_dump.  Global was updated to parse either a MySQL or PostgreSQL section.  BackupAssistant will create the proper derived class object.
> 
> This patch is broken down into a series of smaller patches which may be easier to review.  It's located here https://derocher.org/kde/trac/changeset/31 It goes up to revision 43.
> 
> 
> This addresses bug 227970.
>     https://bugs.kde.org/show_bug.cgi?id=227970
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdepim/runtime/tray/CMakeLists.txt 1105514 
>   trunk/KDE/kdepim/runtime/tray/backup.h 1105514 
>   trunk/KDE/kdepim/runtime/tray/backup.cpp 1105514 
>   trunk/KDE/kdepim/runtime/tray/backupassistant.cpp 1105514 
>   trunk/KDE/kdepim/runtime/tray/global.h 1105514 
>   trunk/KDE/kdepim/runtime/tray/global.cpp 1105514 
> 
> Diff: http://reviewboard.kde.org/r/3350/diff
> 
> 
> Testing
> -------
> 
> Successfully backed up a PostgreSQL database.  I did not test MySQL.
> 
> 
> Thanks,
> 
> Brian
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/