Re: Review Request 123443: Make it possible to call a put job from an IODevice
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123443/ --- (Updated April 26, 2015, 11:43 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Changes --- Submitted with commit 07acbaa5a1be95d346bda4dcb5d3c41f31bd1723 by Aleix Pol to branch master. Repository: kio Description --- Also adopts it when using KIO::AccessManager::put. Otherwise it was doing a ::readAll and then passed the QByteArray around, which isn't very elegant. Diffs - autotests/accessmanagertest.cpp 5e4988f autotests/jobtest.h ef0ec57 autotests/jobtest.cpp b11e9f9 src/core/storedtransferjob.h d3e1106 src/core/storedtransferjob.cpp 7f81d00 src/widgets/accessmanager.cpp 239281e Diff: https://git.reviewboard.kde.org/r/123443/diff/ Testing --- New test for the new method. Tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123443: Make it possible to call a put job from an IODevice
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123443/#review79517 --- Ship it! Ship It! - David Faure On April 22, 2015, 4:13 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123443/ --- (Updated April 22, 2015, 4:13 p.m.) Review request for KDE Frameworks. Repository: kio Description --- Also adopts it when using KIO::AccessManager::put. Otherwise it was doing a ::readAll and then passed the QByteArray around, which isn't very elegant. Diffs - autotests/accessmanagertest.cpp 5e4988f autotests/jobtest.h ef0ec57 autotests/jobtest.cpp b11e9f9 src/core/storedtransferjob.h d3e1106 src/core/storedtransferjob.cpp 7f81d00 src/widgets/accessmanager.cpp 239281e Diff: https://git.reviewboard.kde.org/r/123443/diff/ Testing --- New test for the new method. Tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123443: Make it possible to call a put job from an IODevice
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123443/#review79515 --- I love the idea (QIODevice rocks). But I wonder about the naming. Store was because the data is stored in a QByteArray rather than provided in chunks. But with QIODevice the data might be read incrementally from somewhere (e.g. a file). So it's not really stored into a big memory buffer. So I think this should be a KIO::put() overload. (Or maybe a different name for easier searching, but putIODevice or putFromIODevice isn't great.) - David Faure On April 22, 2015, 4:13 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123443/ --- (Updated April 22, 2015, 4:13 p.m.) Review request for KDE Frameworks. Repository: kio Description --- Also adopts it when using KIO::AccessManager::put. Otherwise it was doing a ::readAll and then passed the QByteArray around, which isn't very elegant. Diffs - autotests/accessmanagertest.cpp 5e4988f autotests/jobtest.h ef0ec57 autotests/jobtest.cpp b11e9f9 src/core/storedtransferjob.h d3e1106 src/core/storedtransferjob.cpp 7f81d00 src/widgets/accessmanager.cpp 239281e Diff: https://git.reviewboard.kde.org/r/123443/diff/ Testing --- New test for the new method. Tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123443: Make it possible to call a put job from an IODevice
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123443/ --- (Updated April 22, 2015, 6:13 p.m.) Review request for KDE Frameworks. Changes --- Fix unit test as pointed out by Albert Vaca, still passes. Repository: kio Description --- Also adopts it when using KIO::AccessManager::put. Otherwise it was doing a ::readAll and then passed the QByteArray around, which isn't very elegant. Diffs (updated) - autotests/accessmanagertest.cpp 5e4988f autotests/jobtest.h ef0ec57 autotests/jobtest.cpp b11e9f9 src/core/storedtransferjob.h d3e1106 src/core/storedtransferjob.cpp 7f81d00 src/widgets/accessmanager.cpp 239281e Diff: https://git.reviewboard.kde.org/r/123443/diff/ Testing --- New test for the new method. Tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123443: Make it possible to call a put job from an IODevice
On April 22, 2015, 9 a.m., Emmanuel Pescosta wrote: src/widgets/accessmanager.cpp, line 255 https://git.reviewboard.kde.org/r/123443/diff/1/?file=362233#file362233line255 IMO you should always verify if the data is readable, because not all lib users are trustworthy (assert isn't meant for input validation of untrustworthy callers - public api) Aleix Pol Gonzalez wrote: I can do it, but QNetworkAccessManager doesn't do that, and I'd say it's better to be as similar to QNAM as possible. Ah ok sry for the noice, didn't know that. Just assumed that they validate it because they mention data must be opened for reading in their API docs. - Emmanuel --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123443/#review79325 --- On April 22, 2015, 1:46 a.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123443/ --- (Updated April 22, 2015, 1:46 a.m.) Review request for KDE Frameworks. Repository: kio Description --- Also adopts it when using KIO::AccessManager::put. Otherwise it was doing a ::readAll and then passed the QByteArray around, which isn't very elegant. Diffs - autotests/accessmanagertest.cpp 5e4988f autotests/jobtest.h ef0ec57 autotests/jobtest.cpp b11e9f9 src/core/storedtransferjob.h d3e1106 src/core/storedtransferjob.cpp 7f81d00 src/widgets/accessmanager.cpp 239281e Diff: https://git.reviewboard.kde.org/r/123443/diff/ Testing --- New test for the new method. Tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123443: Make it possible to call a put job from an IODevice
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123443/#review79325 --- src/widgets/accessmanager.cpp (line 255) https://git.reviewboard.kde.org/r/123443/#comment54156 IMO you should always verify if the data is readable, because not all lib users are trustworthy (assert isn't meant for input validation of untrustworthy callers - public api) - Emmanuel Pescosta On April 22, 2015, 1:46 a.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123443/ --- (Updated April 22, 2015, 1:46 a.m.) Review request for KDE Frameworks. Repository: kio Description --- Also adopts it when using KIO::AccessManager::put. Otherwise it was doing a ::readAll and then passed the QByteArray around, which isn't very elegant. Diffs - autotests/accessmanagertest.cpp 5e4988f autotests/jobtest.h ef0ec57 autotests/jobtest.cpp b11e9f9 src/core/storedtransferjob.h d3e1106 src/core/storedtransferjob.cpp 7f81d00 src/widgets/accessmanager.cpp 239281e Diff: https://git.reviewboard.kde.org/r/123443/diff/ Testing --- New test for the new method. Tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123443: Make it possible to call a put job from an IODevice
On April 22, 2015, 9 a.m., Emmanuel Pescosta wrote: src/widgets/accessmanager.cpp, line 255 https://git.reviewboard.kde.org/r/123443/diff/1/?file=362233#file362233line255 IMO you should always verify if the data is readable, because not all lib users are trustworthy (assert isn't meant for input validation of untrustworthy callers - public api) I can do it, but QNetworkAccessManager doesn't do that, and I'd say it's better to be as similar to QNAM as possible. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123443/#review79325 --- On April 22, 2015, 1:46 a.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123443/ --- (Updated April 22, 2015, 1:46 a.m.) Review request for KDE Frameworks. Repository: kio Description --- Also adopts it when using KIO::AccessManager::put. Otherwise it was doing a ::readAll and then passed the QByteArray around, which isn't very elegant. Diffs - autotests/accessmanagertest.cpp 5e4988f autotests/jobtest.h ef0ec57 autotests/jobtest.cpp b11e9f9 src/core/storedtransferjob.h d3e1106 src/core/storedtransferjob.cpp 7f81d00 src/widgets/accessmanager.cpp 239281e Diff: https://git.reviewboard.kde.org/r/123443/diff/ Testing --- New test for the new method. Tests still pass. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
