----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119530/#review64423 -----------------------------------------------------------
I agree that further features should be developed in conjunction with an application for a real use case. E.g. I'm not sure there's a use case for "all autosavefiles for all apps"... autotests/kautosavefiletest.cpp <https://git.reviewboard.kde.org/r/119530/#comment45001> Well, I meant that there's a QVERIFY here and not in the other two loops that follow. Pretty inconsistent ;) autotests/kautosavefiletest.cpp <https://git.reviewboard.kde.org/r/119530/#comment45002> It can - as long as the function returns void. autotests/kautosavefiletest.cpp <https://git.reviewboard.kde.org/r/119530/#comment45003> unnecessary if before delete. autotests/kautosavefiletest.cpp <https://git.reviewboard.kde.org/r/119530/#comment45004> Don't use bitfield operators on booleans, i.e. remove the '&' here since it's the first use. Interestingly all the following ones are correct. src/lib/io/kautosavefile.h <https://git.reviewboard.kde.org/r/119530/#comment45005> To me kautosavefile was about the KOffice/Calligra use case: saving into a temp file in order to recover the stuff typed by the user and not saved yet, in case of a crash. For backups we also have KBackupFile, in kcoreaddons. This makes me a bit confused about who does what and what's the difference (between KBackupFile and KAutoSaveFile with this (new?) "backup" use case) for an app. Can you clarify? src/lib/io/kautosavefile.h <https://git.reviewboard.kde.org/r/119530/#comment45007> remove spaces around 'x' ;) src/lib/io/kautosavefile.cpp <https://git.reviewboard.kde.org/r/119530/#comment45008> space after ! should be removed src/lib/io/kautosavefile.cpp <https://git.reviewboard.kde.org/r/119530/#comment45009> If .first is a URL, why not use QUrl in the first place as the data type? Otherwise I can see url/path confusions coming up.... src/lib/io/kautosavefile.cpp <https://git.reviewboard.kde.org/r/119530/#comment45010> two spaces before applicationName, it seems src/lib/io/kautosavefile_p.cpp <https://git.reviewboard.kde.org/r/119530/#comment45012> no space after ! src/lib/io/kautosavefile_p.cpp <https://git.reviewboard.kde.org/r/119530/#comment45015> I doubt that schemes can contain special characters ;) I'm even sure they can't. So encoded() isn't really useful (but ok, doesn't really hurt either) src/lib/io/kautosavefile_p.cpp <https://git.reviewboard.kde.org/r/119530/#comment45013> one set of () too many - David Faure On Aug. 11, 2014, 9:22 p.m., Andreas Xavier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119530/ > ----------------------------------------------------------- > > (Updated Aug. 11, 2014, 9:22 p.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. > > > Diffs > ----- > > src/lib/io/kautosavefile_p.cpp PRE-CREATION > src/lib/io/kautosavefile_p.h PRE-CREATION > src/lib/io/kautosavefile.cpp 13a13d7 > src/lib/io/kautosavefile.h 05cc3ae > src/lib/CMakeLists.txt 26eb5a1 > autotests/kautosavefiletest.h cf70f4c > autotests/kautosavefiletest.cpp ec0309e > ! PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/119530/diff/ > > > Testing > ------- > > Ran unit tests. > Ran kdeedu with kanagram. > > > Thanks, > > Andreas Xavier > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
