---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121099/#review70440
---
I am still very unsure about adding all this complexity with two lock files,
some of it in order to add a second feature to KAutoSaveFile ("backups"). That
doesn't really make this patch just a bugfix
The docu patch mentions two usage scenarios, but the second one is about
backups, not autosave. The backup feature belongs to another class (see
KBackup).
I would much rather see a patch for bugfixing the established use case for
KAutoSaveFile (autosaving in order to recover unsaved data after a crash), and
just that, no "let's add another feature while at it".
I might be missing something though, please enlighten me in that case.
autotests/kautosavefiletest.cpp
two spaces before "
autotests/kautosavefiletest.cpp
two spaces after comma
autotests/kautosavefiletest.cpp
same - and there's another dozen that follow ;)
autotests/kautosavefiletest.cpp
this instance isn't deleted anywhere, it seems
autotests/kautosavefiletest.cpp
Better create on stack, for auto cleanup.
src/lib/io/kautosavefile.h
two spaces after removed
src/lib/io/kautosavefile.h
typo: destrctor -> destructor ; comma should be at end of last line
src/lib/io/kautosavefile.h
This is for the first "usage scenario" of the previous paragraph, right?
It's a bit confusing right now, to detail steps without associating them with
the corresponding usage scenario.
Or does this work for both? I don't really understand the second use case.
- David Faure
On Nov. 11, 2014, 1:16 a.m., Jeremy Whiting wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121099/
> ---
>
> (Updated Nov. 11, 2014, 1:16 a.m.)
>
>
> Review request for KDE Frameworks and David Faure.
>
>
> Repository: kcoreaddons
>
>
> Description
> ---
>
> kautosave doesn't work when any app tries to use a second filename, because
> it doesn't filter on filename. The unit tests can be droppped into master to
> show the problem, if you remove the include on line 21.
>
> This patch:
> 1. Adds unit tests to test more behavior mentioned in the header.
> 2. Fixes kautosave working with multple files per application.
> 3. Fixes filenaming brittleness, which would cause kautosave to randomly fail
> when the last 3 randomly generated characters in the filename matched any 3
> consequtive chracters in the managed filename.
>
> Original patch by Andreas Xavier (https://git.reviewboard.kde.org/r/119530)
>
> Items from original review have been fixed.
>
>
> Diffs
> -
>
> src/lib/io/kautosavefile_p.cpp PRE-CREATION
> autotests/kautosavefiletest.h cf70f4c6a4e1f093c431eff6ff25e6f510e84a53
> autotests/kautosavefiletest.cpp ec0309e5fda95e01bbed31bd2fe91f9b7a48bec0
> src/lib/CMakeLists.txt 1dc56270ab5157af706b7745cfa88ae11e16184a
> src/lib/io/kautosavefile.h 05cc3aedc248665c8727ade3b86b524275c013ca
> src/lib/io/kautosavefile.cpp 13a13d7cfb26194be3edf7cbdcd1d39309b79465
> src/lib/io/kautosavefile_p.h PRE-CREATION
>
> Diff: https://git.reviewboard.kde.org/r/121099/diff/
>
>
> Testing
> ---
>
> It builds and the test passes.
>
>
> Thanks,
>
> Jeremy Whiting
>
>
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
