Re: [Libreoffice] [PATCH] Removed never defined _OLD_FILE_IMPL
On 12/04/11 19:44, Christina Roßmanith wrote: Removed last occurences of _USE_UNO, built successfully and pushed. Christina great and thanks for your excellent efforts !!! Noel ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Removed never defined _OLD_FILE_IMPL
Removed last occurences of _USE_UNO, built successfully and pushed. Christina Am 12.04.2011 10:49, schrieb Noel Power: On 11/04/11 15:55, Thorsten Behrens wrote: Noel Power wrote: As far as I understand the code it consists merely of test calls to functions in order to see if they return a valid result. I'm quite sure that you already know whether it has side effects or not... I vote for "it hasn't", but I'm absolutely not sure nah, just seems like some testing to see if uno is bootstrapped and the ucp stuff available, it looks to me especially in the case you removed to be completely useless. Btw if you are feeling frisky and wish to kill other related pieces of uselessness :-) there are the #ifdef _USE_UNO associated blocks that you can see around the hasUno() method ( and others ) that need some removing ( afaics _USE_UNO is defined always ) So we're safe to apply the original patch (with the improvements suggested previously)? I'm pretty sure Christina pushed the patch to do with the _OLD_FILE_IMPL IIRC Christina was going to squash the _USE_UNO pieces too, not sure if that happened yet or not, Christina did you get a chance to do that or should we maybe add it as an easy hack if you didn't have time? Thanks, Noel ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Removed never defined _OLD_FILE_IMPL
Hello On Tue, Apr 12, 2011 at 19:27, Christina Roßmanith wrote: > Is there a way to search for all commits I've done so far? I'll finish that, of course! Maybe using search box in the top-right of the page like this works for you? http://cgit.freedesktop.org/libreoffice/libs-core/log/?qt=author&q=christina Best regards -- Korrawit Pruegsanusak ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Removed never defined _OLD_FILE_IMPL
_USE_UNO: I thought, that I had removed everything #ifdef'ed with _USE_UNO. BUT I've found some occurences in ./basic. Is there a way to search for all commits I've done so far? I'll finish that, of course! Christina Am 12.04.2011 10:49, schrieb Noel Power: On 11/04/11 15:55, Thorsten Behrens wrote: Noel Power wrote: As far as I understand the code it consists merely of test calls to functions in order to see if they return a valid result. I'm quite sure that you already know whether it has side effects or not... I vote for "it hasn't", but I'm absolutely not sure nah, just seems like some testing to see if uno is bootstrapped and the ucp stuff available, it looks to me especially in the case you removed to be completely useless. Btw if you are feeling frisky and wish to kill other related pieces of uselessness :-) there are the #ifdef _USE_UNO associated blocks that you can see around the hasUno() method ( and others ) that need some removing ( afaics _USE_UNO is defined always ) So we're safe to apply the original patch (with the improvements suggested previously)? I'm pretty sure Christina pushed the patch to do with the _OLD_FILE_IMPL IIRC Christina was going to squash the _USE_UNO pieces too, not sure if that happened yet or not, Christina did you get a chance to do that or should we maybe add it as an easy hack if you didn't have time? Thanks, Noel ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Removed never defined _OLD_FILE_IMPL
On 11/04/11 15:55, Thorsten Behrens wrote: Noel Power wrote: As far as I understand the code it consists merely of test calls to functions in order to see if they return a valid result. I'm quite sure that you already know whether it has side effects or not... I vote for "it hasn't", but I'm absolutely not sure nah, just seems like some testing to see if uno is bootstrapped and the ucp stuff available, it looks to me especially in the case you removed to be completely useless. Btw if you are feeling frisky and wish to kill other related pieces of uselessness :-) there are the #ifdef _USE_UNO associated blocks that you can see around the hasUno() method ( and others ) that need some removing ( afaics _USE_UNO is defined always ) So we're safe to apply the original patch (with the improvements suggested previously)? I'm pretty sure Christina pushed the patch to do with the _OLD_FILE_IMPL IIRC Christina was going to squash the _USE_UNO pieces too, not sure if that happened yet or not, Christina did you get a chance to do that or should we maybe add it as an easy hack if you didn't have time? Thanks, Noel ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Removed never defined _OLD_FILE_IMPL
Noel Power wrote: > >As far as I understand the code it consists merely of test calls > >to functions in order to see if they return a valid result. I'm > >quite sure that you already know whether it has side effects or > >not... I vote for "it hasn't", but I'm absolutely not sure > nah, just seems like some testing to see if uno is bootstrapped and > the ucp stuff available, it looks to me especially in the case you > removed to be completely useless. > Btw if you are feeling frisky and wish to kill other related pieces > of uselessness :-) there are the #ifdef _USE_UNO associated blocks > that you can see around the hasUno() method ( and others ) that need > some removing ( afaics _USE_UNO is defined always ) > So we're safe to apply the original patch (with the improvements suggested previously)? Cheers, -- Thorsten pgpKdhvA0AGmc.pgp Description: PGP signature ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Removed never defined _OLD_FILE_IMPL
Hi Christina On 21/03/11 22:28, Christina Roßmanith wrote: nd calls nothing that does that ? As far as I understand the code it consists merely of test calls to functions in order to see if they return a valid result. I'm quite sure that you already know whether it has side effects or not... I vote for "it hasn't", but I'm absolutely not sure nah, just seems like some testing to see if uno is bootstrapped and the ucp stuff available, it looks to me especially in the case you removed to be completely useless. Btw if you are feeling frisky and wish to kill other related pieces of uselessness :-) there are the #ifdef _USE_UNO associated blocks that you can see around the hasUno() method ( and others ) that need some removing ( afaics _USE_UNO is defined always ) Noel ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Removed never defined _OLD_FILE_IMPL
Am 21.03.2011 18:10, schrieb Michael Meeks: On Mon, 2011-03-21 at 13:44 +0100, Christina Roßmanith wrote: I've removed all blocks of code #ifdef'ed by _OLD_FILE_IMPL because it was never defined. It builds successfully. Could someone please review to make sure that nothing was deleted accidentally? @@ -427,7 +425,7 @@ void OslStream::SetSize( sal_uIntPtr nSize ) maFile.setSize( (sal_uInt64)nSize ); } -#endif +//#endif I would fully remove that :-) -if( !hasUno() ) and hasUno has no side-effects ? ie. changes no global / local / object state, and calls nothing that does that ? As far as I understand the code it consists merely of test calls to functions in order to see if they return a valid result. I'm quite sure that you already know whether it has side effects or not... I vote for "it hasn't", but I'm absolutely not sure. Christina ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Removed never defined _OLD_FILE_IMPL
Am 21.03.2011 18:10, schrieb Michael Meeks: On Mon, 2011-03-21 at 13:44 +0100, Christina Roßmanith wrote: I've removed all blocks of code #ifdef'ed by _OLD_FILE_IMPL because it was never defined. It builds successfully. Could someone please review to make sure that nothing was deleted accidentally? @@ -427,7 +425,7 @@ void OslStream::SetSize( sal_uIntPtr nSize ) maFile.setSize( (sal_uInt64)nSize ); } -#endif +//#endif Of course! I would fully remove that :-) -if( !hasUno() ) and hasUno has no side-effects ? ie. changes no global / local / object state, and calls nothing that does that ? Oops, indeed I didn't pay attention that it's not just a variable. I'll check that. That's why I've asked for more eyes :-) Christina ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] Removed never defined _OLD_FILE_IMPL
On Mon, 2011-03-21 at 13:44 +0100, Christina Roßmanith wrote: > I've removed all blocks of code #ifdef'ed by _OLD_FILE_IMPL because it > was never defined. It builds successfully. Could someone please review > to make sure that nothing was deleted accidentally? @@ -427,7 +425,7 @@ void OslStream::SetSize( sal_uIntPtr nSize ) maFile.setSize( (sal_uInt64)nSize ); } -#endif +//#endif I would fully remove that :-) -if( !hasUno() ) and hasUno has no side-effects ? ie. changes no global / local / object state, and calls nothing that does that ? Otherwise it looks perfect to me, go go go ! :-) And a nice cleanup too, Thanks, Michael. -- michael.me...@novell.com <><, Pseudo Engineer, itinerant idiot ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
