Re: Review Request: kio_http: fix keepalive timeout parsing
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102822/
---
(Updated Oct. 13, 2011, 5:59 p.m.)
Review request for kdelibs, Andreas Hartmetz and Dawit Alemayehu.
Changes
---
Small improvements:
. update m_request.keepAliveTimeout only if converted int > 0
. added a break to save while cycles on values we are not interested to
Description
---
Keep-alive header can specify multiple, comma-separated, value pairs.
For example what apache web server normally sends is something like that:
"Keep-Alive: timeout=5, max=99"
Actually kio_http fails to extract timeout value because it assumes
keep-alive header can contain only a single value pair.
In the case of example above what it end up to do is
m_request.keepAliveTimeout = QString("5, max=99").toInt(), that returns 0
(wrong!).
Diffs (updated)
-
kioslave/http/http.cpp 2862707
kioslave/http/parsinghelpers.cpp fc75d68
Diff: http://git.reviewboard.kde.org/r/102822/diff/diff
Testing
---
-Patched code compiles
-Hacked a web server and made tests against following keep-alive header
variants:
"Keep-Alive: timeout=5, max=99"
"Keep-Alive: Timeout=5, max=99" (uppercase 'T')
"Keep-Alive: Timeout=5 , max=99"(extra space before comma)
Thanks,
Andrea Iacovitti
Re: Review Request: facePerm is a KDM option, unrelated to the user setting his face (for other apps)
Hi, > No tabs, 4 spaces instead. > > http://techbase.kde.org/Policies/Kdelibs_Coding_Style Almost the complete main.cpp is using tabs currently (except for KCMUserAccount::decodeImgDrop, which uses 2 spaces) so I used it for the two lines I added. Is that okay, or am I supposed to use 4 spaces nevertheless? Kind regards, Ralf
Re: Review Request: Proxy overhaul Part 6: Proxy Configuration dialog rewrite
> On Oct. 14, 2011, 12:05 a.m., Christoph Feck wrote: > > Nice, work, as always :) Only one question: Do the port spin boxes need the > > "Port:" labels, as seen in the ASCII drawings from bug 147340? Or would it > > clutter the interface? Maybe a "Hostname" and "Port" headline for the two > > columns? > > Dawit Alemayehu wrote: > I guess "Port:" can be added. I simply copied what was in the old > configuration dialog with very minor modifications. I do not have a > preference one way or the other. I will add the port labels to clarify the > interface and make sure users understand the purpose of those inputs. Looks good, but one thing that IMHO doesn't look right is the option-specific entry fields appearing and disappearing within the radio button list as they are selected. Might it look better if the 5 options were in a (full width) combo box at the top, with the area below for the variable fields? - Jonathan --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102802/#review7304 --- On Oct. 14, 2011, 5:32 a.m., Dawit Alemayehu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/102802/ > --- > > (Updated Oct. 14, 2011, 5:32 a.m.) > > > Review request for KDE Base Apps. > > > Description > --- > > This patch completely overhauls the proxy configuration dialog so that it is > much simpler to use and maintain. The major difference between it and the old > configuration dialog is the merging of the configuration dialogs for the > different proxy types into the main proxy dialog and the addition of a new > input for specifying SOCKS proxy settings. See the attached screenshots to > see how the new dialog looks. > > > This addresses bugs 82352, 139789, 142562, 145006, 147340, 164460, 189019, > 190149, 190901, 205594, 258196, and 283226. > http://bugs.kde.org/show_bug.cgi?id=82352 > http://bugs.kde.org/show_bug.cgi?id=139789 > http://bugs.kde.org/show_bug.cgi?id=142562 > http://bugs.kde.org/show_bug.cgi?id=145006 > http://bugs.kde.org/show_bug.cgi?id=147340 > http://bugs.kde.org/show_bug.cgi?id=164460 > http://bugs.kde.org/show_bug.cgi?id=189019 > http://bugs.kde.org/show_bug.cgi?id=190149 > http://bugs.kde.org/show_bug.cgi?id=190901 > http://bugs.kde.org/show_bug.cgi?id=205594 > http://bugs.kde.org/show_bug.cgi?id=258196 > http://bugs.kde.org/show_bug.cgi?id=283226 > > > Diffs > - > > konqueror/settings/kio/CMakeLists.txt e5476c9 > konqueror/settings/kio/envvarproxy.ui a09450d > konqueror/settings/kio/kenvvarproxydlg.h c7d2f5f > konqueror/settings/kio/kenvvarproxydlg.cpp ef5c86c > konqueror/settings/kio/kmanualproxydlg.h 4efa63c > konqueror/settings/kio/kmanualproxydlg.cpp 6760957 > konqueror/settings/kio/kproxydlg.h b2cafb8 > konqueror/settings/kio/kproxydlg.cpp 2f512ce > konqueror/settings/kio/kproxydlg.ui 96ff996 > konqueror/settings/kio/kproxydlgbase.h 2047b90 > konqueror/settings/kio/kproxydlgbase.cpp 9933698 > konqueror/settings/kio/ksaveioconfig.h e895c2c > konqueror/settings/kio/ksaveioconfig.cpp 62852fa > konqueror/settings/kio/manualproxy.ui c69047f > > Diff: http://git.reviewboard.kde.org/r/102802/diff/diff > > > Testing > --- > > - Configure proxy through dialog and validate the results by checking the > settings saved in $KDEHOME/share/config/kioslaverc. > - Reloading the proxy dialog to see the saved information is properly loaded > back again. > > > Screenshots > --- > > No Proxy > http://git.reviewboard.kde.org/r/102802/s/296/ > PAC Proxy > http://git.reviewboard.kde.org/r/102802/s/297/ > System Proxy > http://git.reviewboard.kde.org/r/102802/s/298/ > Manual Proxy > http://git.reviewboard.kde.org/r/102802/s/301/ > > > Thanks, > > Dawit Alemayehu > >
Module layout proposal: Split kdegames-data from kdegames
[CC kde-scm-interest for notification only]
[CC kde-buildsystem for feedback on the proposed build system changes]
[CC kde-packagers for feedback on the implied changes to package layouts]
[@CC: please keep discussion on k-c-d and k-g-d only]
Moin moin,
EXECUTIVE SUMMARY
I propose to move the data files from the kdegames module into a new
kdegames-data module to
1. facilitate the move of the remaining source code to Git (while a
method of storing binary data files in Git efficiently is still being
worked on) and
2. enable developers to fetch and compile the kdegames source without
having to download the data files again.
DETAILED PROPOSAL
kdegames is among the few modules that have not yet switched to Git.
The main concern is that the kdegames source tree contains tons [1] of
binary data files, which Git is known not to handle well. All
discussions about moving to Git (on scm-interest and games-devel) have
just let to bikeshedding about how to handle the binary files with
git. I propose to postpone this specific problem and move on with the
Git transition *now*, especially since the solution I want to propose
has added benefit.
I propose to split a new module kdegames-data from kdegames, meaning that:
1. kdegames-data should be built and installed before kdegames.
2. Any kdegames application will refuse to start when the
corresponding data files have not been installed.
Number 2 is a protective measure because, currently, about all games
won't run correctly or look utterly broken without proper indication
of the problem.
If this proposal is implemented, the kdegames module, that is: the
source code, can move to Git without worrying about the data files.
These can move at any later point in time, when a wise solution has
been worked out by our Git specialists. I'll repeat to get this clear:
I'm not proposing to let the data stay in SVN forever. What I want is
to disentangle the fates of the source code and the data files, for
the benefit of both.
The added benefit of this solution is that distributions will be
encouraged to package data files separately. Because this data (and
esp. its format) changes very seldomly only, developers will not need
to checkout this giant mass of data from our SCM servers, but can
instead use the packages provided by their distribution. The same
holds true for drive-by contributors: They only have to clone a tiny
repo containing the source code of the game which they want to hack
on, without fetching megabytes of data files which they already had
installed.
If this proposal is accepted not later than by the end of next week, I
will be able to implement the following changes before the soft
feature freeze (Thu, October 27):
1. Create the new module in SVN, move data files from the current
kdegames module. The toplevel directory is still divided by
applications, of course.
2. Setup the buildsystem for kdegames-data using
macro_optional_add_subdirectory() as in kdegames, so that data can be
selected for installation on a per-application basis. Each set of
application data will additionally install an empty marker file which
indicates that data is available for this application (something like
${DATA_INSTALL_DIR}/${APP}/hasdata; please suggest better schemes if
there are).
My roadmap also includes the following changes to be implemented
before the hard feature freeze (Thu, November 10):
3. Adjust the buildsystem of kdegames to exclude applications from
compilation when required data files are missing. The exclusion can be
overridden by a documented CMake switch. This is consistent with how
other runtime dependencies of kdegames are handled (see
kdegames/kajongg/CMakeLists.txt).
4. Make all applications abort with a visible warning when data files
are missing.
In both cases, "data files are missing" is detected by checking for
the existence of the marker files installed by kdegames-data (see
point 2). If a developer for some reason wants to use the application
without the data files, he can do so by touching the marker file
manually.
As has been said before, if there are no objections, I would like to
implement this proposal starting from the end of next week (Sat,
October 22), to make sure that the required changes get into 4.8.
Greetings
Stefan
[1]
$ find -type f -regextype posix-egrep -regex
'.*\.(wav|ogg|svg|svgz|jpg|png)' | xargs du -hsc | grep total
86M total
$ du -hsc (*~(.svn|.git)) | grep total
113M total
The latter uses zsh extended glob: (*~(.svn|.git)) matches everything
except .svn or .git
Re: Module layout proposal: Split kdegames-data from kdegames
On Friday 14 October 2011, Stefan Majewsky wrote:
> [CC kde-scm-interest for notification only]
> [CC kde-buildsystem for feedback on the proposed build system changes]
> [CC kde-packagers for feedback on the implied changes to package layouts]
> [@CC: please keep discussion on k-c-d and k-g-d only]
>
> Moin moin,
>
> EXECUTIVE SUMMARY
>
> I propose to move the data files from the kdegames module into a new
> kdegames-data module to
> 1. facilitate the move of the remaining source code to Git (while a
> method of storing binary data files in Git efficiently is still being
> worked on) and
> 2. enable developers to fetch and compile the kdegames source without
> having to download the data files again.
>
> DETAILED PROPOSAL
>
> kdegames is among the few modules that have not yet switched to Git.
> The main concern is that the kdegames source tree contains tons [1] of
> binary data files, which Git is known not to handle well. All
> discussions about moving to Git (on scm-interest and games-devel) have
> just let to bikeshedding about how to handle the binary files with
> git. I propose to postpone this specific problem and move on with the
> Git transition *now*, especially since the solution I want to propose
> has added benefit.
>
> I propose to split a new module kdegames-data from kdegames, meaning that:
>
> 1. kdegames-data should be built and installed before kdegames.
> 2. Any kdegames application will refuse to start when the
> corresponding data files have not been installed.
>
> Number 2 is a protective measure because, currently, about all games
> won't run correctly or look utterly broken without proper indication
> of the problem.
>
> If this proposal is implemented, the kdegames module, that is: the
> source code, can move to Git without worrying about the data files.
> These can move at any later point in time, when a wise solution has
> been worked out by our Git specialists. I'll repeat to get this clear:
> I'm not proposing to let the data stay in SVN forever. What I want is
> to disentangle the fates of the source code and the data files, for
> the benefit of both.
>
> The added benefit of this solution is that distributions will be
> encouraged to package data files separately. Because this data (and
> esp. its format) changes very seldomly only, developers will not need
> to checkout this giant mass of data from our SCM servers, but can
> instead use the packages provided by their distribution.
IMO today with usually broadband internet access this shouldn't be much of a
concern (especially if these files change only rarely).
> The same
> holds true for drive-by contributors: They only have to clone a tiny
> repo containing the source code of the game which they want to hack
> on, without fetching megabytes of data files which they already had
> installed.
Personally I'd say "who cares". At least I don't check anymore how big a
checkout will be before doing the checkout.
> If this proposal is accepted not later than by the end of next week, I
> will be able to implement the following changes before the soft
> feature freeze (Thu, October 27):
>
> 1. Create the new module in SVN, move data files from the current
> kdegames module. The toplevel directory is still divided by
> applications, of course.
I'm not sure I really like this.
The general trend is to split per-application, so there would be e.g. one
repository kolf and one for ksquares.
If data/ goes into one separate repository, this will make it harder once the
sources are split into multiple repositories.
Either each small game would depend on all data files, or each small game
would consist of two packages, one for the data and one for the actual
program.
Or will packager take care of this and create nice packages ?
> 2. Setup the buildsystem for kdegames-data using
> macro_optional_add_subdirectory() as in kdegames, so that data can be
> selected for installation on a per-application basis. Each set of
> application data will additionally install an empty marker file which
> indicates that data is available for this application (something like
> ${DATA_INSTALL_DIR}/${APP}/hasdata; please suggest better schemes if
> there are).
Would it be possible to simply check whether the directory exists ?
${DATA_INSTALL_DIR}/${APP}/Data/ or something like this ?
> My roadmap also includes the following changes to be implemented
> before the hard feature freeze (Thu, November 10):
>
> 3. Adjust the buildsystem of kdegames to exclude applications from
> compilation when required data files are missing. The exclusion can be
> overridden by a documented CMake switch. This is consistent with how
> other runtime dependencies of kdegames are handled (see
> kdegames/kajongg/CMakeLists.txt).
Hmm, not sure.
I'd prefer if it would also build when the data files are not present (since
they are not required for building).
> 4. Make all applications abort with a visible warning when data files
> are mis
Re: Module layout proposal: Split kdegames-data from kdegames
On Fri, Oct 14, 2011 at 9:30 PM, Alexander Neundorf wrote:
> IMO today with usually broadband internet access this shouldn't be much of a
> concern (especially if these files change only rarely).
For all games, the data contributes 400 MB of uncompressable history.
(I think that's the number, although it would be nice if someone who
already ran the kdegames svn2git rules could confirm this.)
And no, broadband internet access is not yet ubiquituous. Even here in
Germany, there is a non-negligible percentage of people on less than 1
MBit/s (sometimes much less). Also, broadband internet access is in a
non-negligible number of cases over UMTS and thus limited by volume
plans, so also the raw download volume matters very much in a lot of
cases.
>> The same
>> holds true for drive-by contributors: They only have to clone a tiny
>> repo containing the source code of the game which they want to hack
>> on, without fetching megabytes of data files which they already had
>> installed.
>
> Personally I'd say "who cares". At least I don't check anymore how big a
> checkout will be before doing the checkout.
I vote for splitting the data not because I want to save some
bandwidth. It's because we got numerous complaints with the initial
plan to include the data in the Git repos. Among others, I
specifically remember Aaron caring about drive-by contributors.
> I'm not sure I really like this.
> The general trend is to split per-application, so there would be e.g. one
> repository kolf and one for ksquares.
> If data/ goes into one separate repository, this will make it harder once the
> sources are split into multiple repositories.
> Either each small game would depend on all data files, or each small game
> would consist of two packages, one for the data and one for the actual
> program.
> Or will packager take care of this and create nice packages ?
Yes, the plan is that each game should be split into two packages.
I've consciously included kde-packagers@ for their feedback on this
plan. My humble impression is that package dependency solvers are fast
enough nowadays to handle multiple packages even for small apps.
Each game depends on its data files only. That's why each set of data
(per application) shall create its own marker file.
macro_optional_add_subdirectory ensures that partial SVN checkouts
continue to work for cases where the developer needs a more current
version of his specific data files.
>> 2. Setup the buildsystem for kdegames-data using
>> macro_optional_add_subdirectory() as in kdegames, so that data can be
>> selected for installation on a per-application basis. Each set of
>> application data will additionally install an empty marker file which
>> indicates that data is available for this application (something like
>> ${DATA_INSTALL_DIR}/${APP}/hasdata; please suggest better schemes if
>> there are).
>
> Would it be possible to simply check whether the directory exists ?
> ${DATA_INSTALL_DIR}/${APP}/Data/ or something like this ?
When I checked last time, directories were not cleaned up correctly by
the uninstall target.
> Hmm, not sure.
> I'd prefer if it would also build when the data files are not present (since
> they are not required for building).
It would be possible to build, but it would require a flag. This is
what packagers want to do (since they probably build kdegames-data
separately), but for the random user, I want to make it obvious that
something's wrong by not compiling about everything.
Greetings
Stefan
Re: Review Request: Proxy overhaul Part 6: Proxy Configuration dialog rewrite
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102802/ --- (Updated Oct. 14, 2011, 8:38 p.m.) Review request for KDE Base Apps. Description --- This patch completely overhauls the proxy configuration dialog so that it is much simpler to use and maintain. The major difference between it and the old configuration dialog is the merging of the configuration dialogs for the different proxy types into the main proxy dialog and the addition of a new input for specifying SOCKS proxy settings. See the attached screenshots to see how the new dialog looks. This addresses bugs 82352, 139789, 142562, 145006, 147340, 164460, 189019, 190149, 190901, 205594, 258196, and 283226. http://bugs.kde.org/show_bug.cgi?id=82352 http://bugs.kde.org/show_bug.cgi?id=139789 http://bugs.kde.org/show_bug.cgi?id=142562 http://bugs.kde.org/show_bug.cgi?id=145006 http://bugs.kde.org/show_bug.cgi?id=147340 http://bugs.kde.org/show_bug.cgi?id=164460 http://bugs.kde.org/show_bug.cgi?id=189019 http://bugs.kde.org/show_bug.cgi?id=190149 http://bugs.kde.org/show_bug.cgi?id=190901 http://bugs.kde.org/show_bug.cgi?id=205594 http://bugs.kde.org/show_bug.cgi?id=258196 http://bugs.kde.org/show_bug.cgi?id=283226 Diffs - konqueror/settings/kio/CMakeLists.txt e5476c9 konqueror/settings/kio/envvarproxy.ui a09450d konqueror/settings/kio/kenvvarproxydlg.h c7d2f5f konqueror/settings/kio/kenvvarproxydlg.cpp ef5c86c konqueror/settings/kio/kmanualproxydlg.h 4efa63c konqueror/settings/kio/kmanualproxydlg.cpp 6760957 konqueror/settings/kio/kproxydlg.h b2cafb8 konqueror/settings/kio/kproxydlg.cpp 2f512ce konqueror/settings/kio/kproxydlg.ui 96ff996 konqueror/settings/kio/kproxydlgbase.h 2047b90 konqueror/settings/kio/kproxydlgbase.cpp 9933698 konqueror/settings/kio/ksaveioconfig.h e895c2c konqueror/settings/kio/ksaveioconfig.cpp 62852fa konqueror/settings/kio/manualproxy.ui c69047f Diff: http://git.reviewboard.kde.org/r/102802/diff/diff Testing --- - Configure proxy through dialog and validate the results by checking the settings saved in $KDEHOME/share/config/kioslaverc. - Reloading the proxy dialog to see the saved information is properly loaded back again. Screenshots (updated) --- No Proxy http://git.reviewboard.kde.org/r/102802/s/296/ PAC Proxy http://git.reviewboard.kde.org/r/102802/s/297/ System Proxy http://git.reviewboard.kde.org/r/102802/s/298/ Manual Proxy http://git.reviewboard.kde.org/r/102802/s/301/ Alternate dialog with all options visible http://git.reviewboard.kde.org/r/102802/s/303/ Thanks, Dawit Alemayehu
Re: Review Request: Improved proxy data caching in KProtocolManager
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102840/#review7350 --- This review has been submitted with commit 0ea92d0b09f282d745de9444ed5c0c9decb06db0 by Dawit Alemayehu to branch KDE/4.7. - Commit Hook On Oct. 13, 2011, 7:30 p.m., Dawit Alemayehu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/102840/ > --- > > (Updated Oct. 13, 2011, 7:30 p.m.) > > > Review request for kdelibs. > > > Description > --- > > The attached patch attempts to improve proxy lookup performance by using > QCache to cache the proxy data for the last 100 (QCache default) lookups > instead of caching just the last lookup. The current approach, where only the > last lookup is cached, is useless whenever two consecutive requests for proxy > data are for different URLs. > > > Diffs > - > > kio/kio/kprotocolmanager.cpp 92519e4 > > Diff: http://git.reviewboard.kde.org/r/102840/diff/diff > > > Testing > --- > > > Thanks, > > Dawit Alemayehu > >
