Re: Review Request: Do not terminate threads

2011-08-22 Thread David Faure
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/#review5910 --- Ship it! Hmm, indeed. I had another version of these patches

Re: Review Request: Do not terminate threads

2011-08-22 Thread Commit Hook
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/#review5914 --- This review has been submitted with commit

Re: Review Request: Do not terminate threads

2011-08-22 Thread Commit Hook
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/#review5915 --- This review has been submitted with commit

Re: Review Request: Do not terminate threads

2011-08-21 Thread David Faure
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/#review5875 --- I think this patch is almost ready, but Dawit's comment seems

Re: Review Request: Do not terminate threads

2011-08-21 Thread Albert Astals Cid
On Aug. 21, 2011, 10:29 a.m., David Faure wrote: I think this patch is almost ready, but Dawit's comment seems right about an improvement that should be done to it: - Move all the preemptive lookup logic from the thread's run function to HostInfo::lookupHost. [No need to create a

Re: Review Request: Do not terminate threads

2011-08-15 Thread Thomas Zander
On Sunday 14 August 2011 23.05.41 Albert Astals Cid wrote: #2 Your patch has several issues i mentioned there I noticed those too, and I wanted to just say that I'd trust David and Thiago on these concepts any day. Maybe we can just use the structure they suggested (and Albert coded) and move

Re: Review Request: Do not terminate threads

2011-08-14 Thread Albert Astals Cid
On Aug. 12, 2011, 3:45 a.m., Dawit Alemayehu wrote: #1. Doesn't this approach have similar issues to the one that forced me to change the previous QtConcurrent::run based implementation ? That is, doesn't the use of a single thread expose this function to lookup requests backlog and

Re: Review Request: Do not terminate threads

2011-08-11 Thread David Faure
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/#review5623 --- Good job, this is tricky code indeed. Some comments below.

Re: Review Request: Do not terminate threads

2011-08-11 Thread Albert Astals Cid
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/ --- (Updated Aug. 11, 2011, 10:32 a.m.) Review request for kdelibs and Dawit

Re: Review Request: Do not terminate threads

2011-08-11 Thread Albert Astals Cid
On Aug. 11, 2011, 9:41 a.m., David Faure wrote: kio/kio/hostinfo.cpp, line 236 http://git.reviewboard.kde.org/r/102179/diff/5/?file=31680#file31680line236 Maybe move nameLookupThread as a member of hostInfoAgentPrivate, to avoid multiplying the global statics (and therefore have

Re: Review Request: Do not terminate threads

2011-08-11 Thread Albert Astals Cid
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/ --- (Updated Aug. 11, 2011, 10:34 a.m.) Review request for kdelibs and Dawit

Re: Review Request: Do not terminate threads

2011-08-11 Thread Thiago Macieira
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/#review5636 --- kio/kio/hostinfo.cpp

Re: Review Request: Do not terminate threads

2011-08-11 Thread David Faure
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/#review5653 --- kio/kio/hostinfo.cpp

Re: Review Request: Do not terminate threads

2011-08-11 Thread Albert Astals Cid
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/ --- (Updated Aug. 11, 2011, 10:10 p.m.) Review request for kdelibs and Dawit

Re: Review Request: Do not terminate threads

2011-08-11 Thread Dawit Alemayehu
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/#review5654 --- #1. Doesn't this approach have similar issues to the one that

Re: Review Request: Do not terminate threads

2011-08-06 Thread Albert Astals Cid
On Dijous 04 Agost 2011 15:28:49 Dawit A wrote: On Thu, Aug 4, 2011 at 5:31 AM, Albert Astals Cid tsdg...@terra.es wrote: This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/ On August 4th, 2011, 3:19 a.m., *Dawit Alemayehu* wrote:

Review Request: Do not terminate threads

2011-08-06 Thread Dawit A
On Sat, Aug 6, 2011 at 7:46 AM, Albert Astals Cid aa...@kde.org wrote: On Dijous 04 Agost 2011 15:28:49 Dawit A wrote: On Thu, Aug 4, 2011 at 5:31 AM, Albert Astals Cid tsdg...@terra.es wrote:    This is an automatically generated e-mail. To reply, visit:

Re: Review Request: Do not terminate threads

2011-08-04 Thread Thiago Macieira
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/#review5371 --- QHostInfo is already threaded. This code is unnecessary today

Re: Review Request: Do not terminate threads

2011-08-04 Thread Albert Astals Cid
On Aug. 4, 2011, 7:14 a.m., Thiago Macieira wrote: QHostInfo is already threaded. This code is unnecessary today and should be removed. If we remove the thread we need to introduce a nested eventloop since that method is public and supposed to be blocking. Is that OK? - Albert

Re: Review Request: Do not terminate threads

2011-08-04 Thread Thiago Macieira
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/#review5390 --- Ok, then there's something wrong. QHostInfo has had a blocking

Re: Review Request: Do not terminate threads

2011-08-04 Thread Albert Astals Cid
On Aug. 4, 2011, 1:11 p.m., Thiago Macieira wrote: Ok, then there's something wrong. QHostInfo has had a blocking method since Qt 4.0. Can not use it since we have a timeout parameter. Thus we either need the thread (to use the blocking method and forget about the thread when the timeout

Re: Review Request: Do not terminate threads

2011-08-04 Thread Thiago Macieira
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/#review5396 --- That makes sense. Then we need only one thread, one that runs

Re: Review Request: Do not terminate threads

2011-08-04 Thread Dawit A
On Thu, Aug 4, 2011 at 5:31 AM, Albert Astals Cid tsdg...@terra.es wrote: This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/ On August 4th, 2011, 3:19 a.m., *Dawit Alemayehu* wrote: I do not like this patch for the very reason you stated.

Re: Review Request: Do not terminate threads

2011-08-04 Thread Dawit A
On Thu, Aug 4, 2011 at 9:11 AM, Thiago Macieira thi...@kde.org wrote: This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/ Ok, then there's something wrong. QHostInfo has had a blocking method since Qt 4.0. Thiago, you must have forgotten

Re: Review Request: Do not terminate threads

2011-08-03 Thread Dawit Alemayehu
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/#review5369 --- I do not like this patch for the very reason you stated. I do

Re: Review Request: Do not terminate threads

2011-08-02 Thread Albert Astals Cid
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/ --- (Updated Aug. 2, 2011, 11:29 a.m.) Review request for kdelibs and Dawit

Re: Review Request: Do not terminate threads

2011-08-02 Thread Albert Astals Cid
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/ --- (Updated Aug. 2, 2011, 11:29 a.m.) Review request for kdelibs and Dawit